Summary: Various tidying up of linting code.
Test Plan: `arc lint` and `arc unit` still pass.
Reviewers: chad, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9625
Summary: Fixes T5300. Currently, if a dead symbolic link is linted, all kinds of errors will be thrown by most linters because they will try to read the (non-existent) file contents. Instead, let's not lint symbolic links by default. In the case that the target of a symbolic link is inside the working copy, then it should be being linted anyway.
Test Plan: Created a symbolic link and verified that it wasn't linted (by any linter other than the `ArcanistFilenameLinter`).
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T5300
Differential Revision: https://secure.phabricator.com/D9448
Summary: Ref T2039. This isn't exhaustive, but moves things forward by a decent chunk.
Test Plan: Used `arc linters` and read the messages.
Reviewers: chad, btrahan, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T2039
Differential Revision: https://secure.phabricator.com/D9056
Summary:
Ref T3186. Ref T2039. Ref T3771. A few effects here:
# Expose PHPCS as a `.arclint` linter.
# Turn PHPCS into an ArcanistExternalLinter linter.
# Add test coverage for PHPCS.
# Add a `severity.rules` option to `.arclint`. Some linters have very explicit builtin severities ("error", "warning") but their meanings are different from how arc interprets these terms. For example, PHPCS raises "wrong indentation level" as an "error". You can already use the "severity" map to adjust individual rules, but if you want to adjust an entire linter it's currently difficult. This rule map makes it easy. There's substantial precedent for this in other linters, notably all the Python linters.
For `severity.rules`, for example, this will turn all PHPCS "errors" into warnings, and all of its warnings into advice:
"severity.rules" : {
"(^PHPCS\\.E\\.)" : "warning",
"(^PHPCS\\.W\\.)" : "advice"
}
The user can use `severity` (or more rules) to get additional granularity adjustments if they desire.
Test Plan: 5bb919bc3a
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, ajtrichards
Maniphest Tasks: T2039, T3186, T3771
Differential Revision: https://secure.phabricator.com/D6830
Summary:
Ref T3186. Ref T2039. Continues work on readying linters for `.arclint`.
- **Ruby**: Make this an ExternalLinter.
- **Priority**: Currently, linters have an implicit "correct" order (notably, the "NoLint" linter needs to run before other linters). Make this explicit by introducing `getLinterPriority()`.
- **Binaries**: Currently, linters manually reject binary files. Instead, reject binary files by default (linters can override this if they do want to lint binary files).
- **Deleted Files**: Currently, linters manually reject deleted files (usually in engines). Instead, reject deleted files by default (linters can override this).
- **Severity**: Move this `.arclint` config option up to top level.
- **willLintPaths()**: This method is abstract, but almost all linters provide a trivial implementation. Provide a trivial implementation in the base class.
- **getLintSeverityMap()/getLintNameMap()**: A bunch of linters have empty implementations; these are redundant. Remove them.
- **Spelling**: clean up some dead / test-only / unconventional code.
- **`.arclint`**: Allow the filename, generated, nolint, text, spelling and ruby linters to be configured via `.arclint`.
Test Plan:
458beca3d6
Ran unit tests.
Reviewers: btrahan
Reviewed By: btrahan
CC: Firehed, aran
Maniphest Tasks: T2039, T3186
Differential Revision: https://secure.phabricator.com/D6805
Summary:
Ref T2039. That task has a bunch of discussion, but basically we do a poor job of serving the midrange of lint configuration right now.
If you have something simple, the default linters work.
If you have something complex, building your own engine lets you do whatever you want.
But many users want something in between, which isn't really well accommodated. The idea is to let you write a `.arclint` file, which looks something like this:
{
"linters" : {
"css" : {
"type" : "csslint",
"include" : "(\.css$)",
"exclude" : "(^externals/)",
"bin" : "/usr/local/bin/csslint"
},
"js" : {
"type" : "jshint",
"include" : "(\.js$)",
"exclude" : "(^externals/)",
"bin" : "support/bin/jshint",
"interpreter" : "/usr/local/bin/node"
}
}
}
...which will provide a bunch of common options around lint severity, interpreter and binary locaitons, included and excluded files, etc.
This implements some basics, and very rough support in the Filename linter.
Test Plan:
Generated a `.arclint` file and saw it apply filename lint correctly. Used `debug` mode and tried invalid regexps.
{
"debug" : true,
"linters" : {
"filename" : {
"type" : "filename",
"exclude" : ["@^externals/@"]
}
}
}
Next steps include:
- Provide an external linter archetype (T3186) and expose a common set of configuration here ("bin", "interpreter", "flags", "severity").
- Provide a `.arcunit` file which works similarly (it can probably be simpler).
Reviewers: btrahan, Firehed
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2039
Differential Revision: https://secure.phabricator.com/D6797
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in unit tests and LICENSE file.
Reviewers: epriestley, btrahan, edward
Reviewed By: epriestley
CC: aran, Korvin, davidrecordon
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3881
