Linter: Make RuleClass().name static#1268
Conversation
citizen428
force-pushed
the
issue-1264
branch
2 times, most recently
from
3437e60 to
48db805
Compare
|
|
||
| const RuleClass = (this.linter as any).rules.find((rule: any) => { | ||
| const instance = new rule() | ||
| const ruleClass = (this.linter as any).rules.find( |
There was a problem hiding this comment.
On top of changing the find I also renamed RuleClass to ruleClass. I think it's confusing to name a variable referencing a class in camel-case like the constructor function.
| } | ||
|
|
||
| async processFiles(files: string[], formatOption: FormatOption = 'detailed', context?: ProcessingContext): Promise<ProcessingResult> { | ||
| async processFiles( |
There was a problem hiding this comment.
I didn't catch that my editor auto-formatted this. Let me know if you want me to roll this back. Same for other format changes.
There was a problem hiding this comment.
If you can easily revert it, then that would be great! On that note, I guess we should also enforce formatting for JavaScript/TypeScript files.
|
@marcoroth OK, I believe this is ready now. I know you also mentioned potentially doing something similar for |
|
I think we also need to update the docs: https://herb-tools.dev/projects/linter#custom-rules Also wondering if we can easily provide a deprecation message if we see somebody is trying to define |
Done in 153de47, please check.
I pushed an approach for that in b5c5020, let me know what you think. I opted for an error and termination to force people into updating their custom rules,. but maybe that's too aggressive. |
marcoroth
left a comment
marcoroth
left a comment
There was a problem hiding this comment.
Thank you @citizen428, I really appreciate the thoroughness! 🙏🏼
This pull request refactors the linter by accessing the `static type` field directly on rule classes instead of instantiating rules and reading it from `rule.constructor`. The instance-level type guards (`isLexerRule`, `isSourceRule`, `isParserRule`) are replaced with class-level equivalents (`isLexerRuleClass`, `isSourceRuleClass`, `isParserRuleClass`) that take a `ruleClass` rather than a Rule instance. It also extracts a `findRuleClass(ruleName)` helper to deduplicate the six occurrences of `this.rules.find(rule => rule.ruleName === ...)`, and renames `RuleClass` local variables to `ruleClass` to follow camelCase conventions for non-type identifiers. Follow up on #1268
2 participants
Closes #1264
This PR refactors the linter code to use a static
ruleNameproperty instead ofnameon the instance.