This class provides an adapter for lessc.
Details
- Reviewers
epriestley chad - Group Reviewers
Blessed Reviewers - Commits
- rARC88ff113f921c: Add a LESS linter.
Wrote and executed unit tests.
Diff Detail
- Repository
- rARC Arcanist
- Branch
- lessc
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 266 Build 266: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
One issue, maybe? Is getLinterConfigurationOptions() not actually required to use thsoe options in .arclint? I'd expect an exception if you try to specify them without overriding it.
src/lint/linter/ArcanistLesscLinter.php | ||
---|---|---|
6–7 | We just render these blocks with remarkup: [[ https://blahblah | lessc ]] | |
79–80 | I would expect that you would need to implement getLinterConfigurationOptions() to support these? Maybe we should prefix them too (lessc.strict-math). |
I don't quite understand your comment. How are options passed from .arclint to the linters? I'm not sure myself.
I don't quite understand your comment. How are options passed from .arclint to the linters? I'm not sure myself.
ArcanistConfigurationDrivenLintEngine loads the configuration engine near the top of buildLinters(), near line 27:
It does a top-level typecheck for the file format (basically, it should contain a map of linter specifications):
PhutilTypeSpec::checkMap( $config, array( 'linters' => 'map<string, map<string, wild>>', 'debug' => 'optional bool', ));
Then it goes through linters and typechecks each one of those, by calling getLinterConfigurationOptions(). This is implemented by:
- ArcanistLinter (adds severity, severity.rules)
- ArcanistExternalLinter (adds bin, flags, interpreter)
- optionally the concrete linter class itself, to add more flags
Then the actual typecheck happens, near line 57, which checks type, include and exclude:
PhutilTypeSpec::checkMap( $spec, array( 'type' => 'string', 'include' => 'optional string | list<string>', 'exclude' => 'optional string | list<string>', ) + $more);
So the expectation is that if you have a .arclint file and add arbitrary configuration to a linter section, you'll get an exception when you run lint if you didn't actually specify that the config keys are known (some day, this should be a little prettier):
$ arc lint --rev HEAD^ Exception Got unexpected parameters: quack (Run with --trace for a full exception trace.)
So to add options strict-math and strict-units, I'd expect you to need something like (I namespaced the option names here):
public function getLinterConfigurationOptions() { return parent::getLinterConfigurationOptions() + array( 'lessc.strict-math' => 'optional bool', 'lessc.strict-units' => 'optional bool', ); }
...otherwise, you'll get that exception if you specify these in .arclint and run lint.
- Provide an implementation for getLinterConfigurationOptions.
- Prefix configuration keys with lessc..
Closed by commit rARC88ff113f921c (authored by @joshuaspence, committed by @epriestley).