Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions javascript/packages/linter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ class NoDivTagsVisitor extends BaseRuleVisitor {
}

export default class NoDivTagsRule extends ParserRule {
name = "no-div-tags"
static ruleName = "no-div-tags"

check(result, context) {
const visitor = new NoDivTagsVisitor(this.name, context)
Expand Down Expand Up @@ -566,7 +566,7 @@ class NoInlineStylesVisitor extends BaseRuleVisitor {
}

export default class NoInlineStylesRule {
name = "no-inline-styles"
static ruleName = "no-inline-styles"

check(parseResult, context) {
const visitor = new NoInlineStylesVisitor(this.name, context)
Expand All @@ -590,7 +590,7 @@ You can override the `defaultConfig` getter to customize these defaults, as show
**Rule Properties:**

- `static type` - Optional, defaults to `"parser"`. Can be `"parser"`, `"lexer"`, or `"source"`
- `name` - Required, the rule identifier used in configuration and output
- `static ruleName` - Required, the rule identifier used in configuration and output
- `check()` - Required, the method that checks for offenses
- `defaultConfig` - Optional, returns the default configuration for the rule
- `isEnabled()` - Optional, dynamically determines if the rule should run
Expand Down
12 changes: 5 additions & 7 deletions javascript/packages/linter/src/cli/file-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,13 @@ export class FileProcessor {
private isRuleAutocorrectable(ruleName: string): boolean {
if (!this.linter) return false

const RuleClass = (this.linter as any).rules.find((rule: any) => {
const instance = new rule()
const ruleClass = (this.linter as any).rules.find(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

(rule: any) => rule.ruleName === ruleName
)

return instance.name === ruleName
})
if (!ruleClass) return false

if (!RuleClass) return false

return RuleClass.autocorrectable === true
return ruleClass.autocorrectable === true
}

async processFiles(files: string[], formatOption: FormatOption = 'detailed', context?: ProcessingContext): Promise<ProcessingResult> {
Expand Down
25 changes: 20 additions & 5 deletions javascript/packages/linter/src/custom-rule-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,16 @@ export class CustomRuleLoader {
const cacheBustedUrl = `${fileUrl}?t=${Date.now()}`
const module = await import(cacheBustedUrl)

if (module.default && this.isValidRuleClass(module.default)) {
if (this.isValidRuleClass(module.default)) {
Comment thread
marcoroth marked this conversation as resolved.
return [module.default]
}

if (this.hasDeprecatedNameProperty(module.default)) {
Comment thread
marcoroth marked this conversation as resolved.
const name = new module.default().name
console.error(`Error: Custom rule in "${filePath}" sets 'name' as an instance property, which is no longer supported. Use 'static ruleName = "${name}"' instead.`)
process.exit(1)
}

if (!this.silent) {
console.warn(`Warning: No valid default export found in "${filePath}". Custom rules must use default export.`)
}
Expand All @@ -96,12 +102,22 @@ export class CustomRuleLoader {
* Type guard to check if an export is a valid rule class
*/
private isValidRuleClass(value: any): value is RuleClass {
if (!value) return false
if (typeof value !== 'function') return false
if (!value.prototype) return false

const instance = new value()
return typeof value.ruleName === 'string' && typeof value.prototype.check === 'function'
}

return typeof instance.check === 'function' && typeof instance.name === 'string'
/**
* Check for usage of deprecated 'name' property instead of 'static ruleName'
*/
private hasDeprecatedNameProperty(value: any): boolean {
if (!value) return false
if (typeof value !== 'function') return false

const instance = new value()
return Object.prototype.hasOwnProperty.call(instance, 'name')
}

/**
Expand Down Expand Up @@ -142,8 +158,7 @@ export class CustomRuleLoader {
for (const filePath of ruleFiles) {
const rules = await this.loadRuleFile(filePath)
for (const RuleClass of rules) {
const instance = new RuleClass()
const ruleName = instance.name
const ruleName = RuleClass.ruleName

if (seenNames.has(ruleName)) {
const firstPath = seenNames.get(ruleName)!
Expand Down
46 changes: 16 additions & 30 deletions javascript/packages/linter/src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,9 @@ export class Linter {
): RuleClass[] {
return allRules.filter(ruleClass => {
const instance = new ruleClass()
const ruleName = instance.name

const defaultEnabled = instance.defaultConfig?.enabled ?? DEFAULT_RULE_CONFIG.enabled
const userRuleConfig = userRulesConfig?.[ruleName]
const userRuleConfig = userRulesConfig?.[ruleClass.ruleName]

if (userRuleConfig !== undefined) {
return userRuleConfig.enabled !== false
Expand Down Expand Up @@ -193,13 +192,15 @@ export class Linter {
source: string,
context?: Partial<LintContext>
): UnboundLintOffense[] {
const ruleName = rule.ruleName

if (this.config && context?.fileName) {
if (!this.config.isRuleEnabledForPath(rule.name, context.fileName)) {
if (!this.config.isRuleEnabledForPath(ruleName, context.fileName)) {
return []
}
}

if (context?.fileName && !this.config?.linter?.rules?.[rule.name]?.exclude) {
if (context?.fileName && !this.config?.linter?.rules?.[ruleName]?.exclude) {
const defaultExclude = rule.defaultConfig?.exclude ?? DEFAULT_RULE_CONFIG.exclude

if (defaultExclude && defaultExclude.length > 0) {
Expand Down Expand Up @@ -335,7 +336,7 @@ export class Linter {
const herbDisableCache = new Map<number, string[]>()

if (hasParserErrors) {
const hasParserRule = this.rules.find(RuleClass => (new RuleClass()).name === "parser-no-errors")
const hasParserRule = this.rules.find(RuleClass => RuleClass.ruleName === "parser-no-errors")

if (hasParserRule) {
const rule = new ParserNoErrorsRule()
Expand All @@ -355,15 +356,11 @@ export class Linter {

context = {
...context,
validRuleNames: this.getAvailableRules().map(RuleClass => new RuleClass().name),
validRuleNames: this.getAvailableRules().map(RuleClass => RuleClass.ruleName),
ignoredOffensesByLine
}

const regularRules = this.rules.filter(RuleClass => {
const rule = new RuleClass()

return rule.name !== "herb-disable-comment-unnecessary"
})
const regularRules = this.rules.filter(RuleClass => RuleClass.ruleName !== "herb-disable-comment-unnecessary")

for (const RuleClass of regularRules) {
const rule = new RuleClass()
Expand All @@ -379,11 +376,11 @@ export class Linter {
}

const unboundOffenses = this.executeRule(rule, parseResult, lexResult, source, context)
const boundOffenses = this.bindSeverity(unboundOffenses, rule.name)
const boundOffenses = this.bindSeverity(unboundOffenses, RuleClass.ruleName)

const { kept, ignored, wouldBeIgnored } = this.filterOffenses(
boundOffenses,
rule.name,
RuleClass.ruleName,
ignoredOffensesByLine,
herbDisableCache,
context?.ignoreDisableComments
Expand All @@ -394,17 +391,13 @@ export class Linter {
this.offenses.push(...kept)
}

const unnecessaryRuleClass = this.rules.find(RuleClass => {
const rule = new RuleClass()

return rule.name === "herb-disable-comment-unnecessary"
})
const unnecessaryRuleClass = this.rules.find(RuleClass => RuleClass.ruleName === "herb-disable-comment-unnecessary")

if (unnecessaryRuleClass) {
const unnecessaryRule = new unnecessaryRuleClass() as ParserRule
const parseResult = this.parseCache.get(source, unnecessaryRule.parserOptions)
const unboundOffenses = unnecessaryRule.check(parseResult, context)
const boundOffenses = this.bindSeverity(unboundOffenses, unnecessaryRule.name)
const boundOffenses = this.bindSeverity(unboundOffenses, unnecessaryRuleClass.ruleName)

this.offenses.push(...boundOffenses)
}
Expand Down Expand Up @@ -444,10 +437,7 @@ export class Linter {
* @returns Array of offenses with severity bound
*/
protected bindSeverity(unboundOffenses: UnboundLintOffense[], ruleName: string): LintOffense[] {
const RuleClass = this.rules.find(rule => {
const instance = new rule()
return instance.name === ruleName
})
const RuleClass = this.rules.find(rule => rule.ruleName === ruleName)

if (!RuleClass) {
return unboundOffenses.map(offense => ({
Expand Down Expand Up @@ -487,11 +477,7 @@ export class Linter {
const sourceOffenses: LintOffense[] = []

for (const offense of lintResult.offenses) {
const RuleClass = this.rules.find(rule => {
const instance = new rule()

return instance.name === offense.rule
})
const RuleClass = this.rules.find(rule => rule.ruleName === offense.rule)

if (!RuleClass) continue

Expand All @@ -512,7 +498,7 @@ export class Linter {
const parseResult = this.parseCache.get(currentSource)

for (const offense of parserOffenses) {
const RuleClass = this.rules.find(rule => new rule().name === offense.rule)
const RuleClass = this.rules.find(rule => rule.ruleName === offense.rule)

if (!RuleClass) {
unfixed.push(offense)
Expand Down Expand Up @@ -579,7 +565,7 @@ export class Linter {
})

for (const offense of sortedSourceOffenses) {
const RuleClass = this.rules.find(rule => new rule().name === offense.rule)
const RuleClass = this.rules.find(rule => rule.ruleName === offense.rule)

if (!RuleClass) {
unfixed.push(offense)
Expand Down
4 changes: 2 additions & 2 deletions javascript/packages/linter/src/rules/erb-comment-syntax.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ERBCommentSyntaxVisitor extends BaseRuleVisitor<ERBCommentSyntaxAutofixCon

export class ERBCommentSyntax extends ParserRule<ERBCommentSyntaxAutofixContext> {
static autocorrectable = true
name = "erb-comment-syntax"
static ruleName = "erb-comment-syntax"

get defaultConfig(): FullRuleConfig {
return {
Expand All @@ -44,7 +44,7 @@ export class ERBCommentSyntax extends ParserRule<ERBCommentSyntaxAutofixContext>
}

check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense<ERBCommentSyntaxAutofixContext>[] {
const visitor = new ERBCommentSyntaxVisitor(this.name, context)
const visitor = new ERBCommentSyntaxVisitor(this.ruleName, context)

visitor.visit(result.value)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class ERBNoCaseNodeChildrenVisitor extends BaseRuleVisitor {
}

export class ERBNoCaseNodeChildrenRule extends ParserRule {
name = "erb-no-case-node-children"
static ruleName = "erb-no-case-node-children"

get defaultConfig(): FullRuleConfig {
return {
Expand All @@ -59,7 +59,7 @@ export class ERBNoCaseNodeChildrenRule extends ParserRule {
}

check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
const visitor = new ERBNoCaseNodeChildrenVisitor(this.name, context)
const visitor = new ERBNoCaseNodeChildrenVisitor(this.ruleName, context)

visitor.visit(result.value)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ERBNoConditionalHTMLElementRuleVisitor extends BaseRuleVisitor {
}

export class ERBNoConditionalHTMLElementRule extends ParserRule {
name = "erb-no-conditional-html-element"
static ruleName = "erb-no-conditional-html-element"

get defaultConfig(): FullRuleConfig {
return {
Expand All @@ -44,7 +44,7 @@ export class ERBNoConditionalHTMLElementRule extends ParserRule {
}

check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
const visitor = new ERBNoConditionalHTMLElementRuleVisitor(this.name, context)
const visitor = new ERBNoConditionalHTMLElementRuleVisitor(this.ruleName, context)

visitor.visit(result.value)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class ERBNoConditionalOpenTagRuleVisitor extends BaseRuleVisitor {
}

export class ERBNoConditionalOpenTagRule extends ParserRule {
name = "erb-no-conditional-open-tag"
static ruleName = "erb-no-conditional-open-tag"

get defaultConfig(): FullRuleConfig {
return {
Expand All @@ -28,7 +28,7 @@ export class ERBNoConditionalOpenTagRule extends ParserRule {
}

check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
const visitor = new ERBNoConditionalOpenTagRuleVisitor(this.name, context)
const visitor = new ERBNoConditionalOpenTagRuleVisitor(this.ruleName, context)

visitor.visit(result.value)

Expand Down
4 changes: 2 additions & 2 deletions javascript/packages/linter/src/rules/erb-no-empty-tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class ERBNoEmptyTagsVisitor extends BaseRuleVisitor {
}

export class ERBNoEmptyTagsRule extends ParserRule {
name = "erb-no-empty-tags"
static ruleName = "erb-no-empty-tags"

get defaultConfig(): FullRuleConfig {
return {
Expand All @@ -32,7 +32,7 @@ export class ERBNoEmptyTagsRule extends ParserRule {
}

check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
const visitor = new ERBNoEmptyTagsVisitor(this.name, context)
const visitor = new ERBNoEmptyTagsVisitor(this.ruleName, context)

visitor.visit(result.value)

Expand Down
4 changes: 2 additions & 2 deletions javascript/packages/linter/src/rules/erb-no-extra-newline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class ERBNoExtraNewLineVisitor extends BaseSourceRuleVisitor<ERBNoExtraNewLineAu

export class ERBNoExtraNewLineRule extends SourceRule {
static autocorrectable = true
name = "erb-no-extra-newline"
static ruleName = "erb-no-extra-newline"

get defaultConfig(): FullRuleConfig {
return {
Expand All @@ -51,7 +51,7 @@ export class ERBNoExtraNewLineRule extends SourceRule {
}

check(source: string, context?: Partial<LintContext>): UnboundLintOffense[] {
const visitor = new ERBNoExtraNewLineVisitor(this.name, context)
const visitor = new ERBNoExtraNewLineVisitor(this.ruleName, context)

visitor.visit(source)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class ERBNoExtraWhitespaceInsideTagsVisitor extends BaseRuleVisitor<ERBNoExtraWh

export class ERBNoExtraWhitespaceRule extends ParserRule<ERBNoExtraWhitespaceAutofixContext> {
static autocorrectable = true
name = "erb-no-extra-whitespace-inside-tags"
static ruleName = "erb-no-extra-whitespace-inside-tags"

get defaultConfig(): FullRuleConfig {
return {
Expand All @@ -107,7 +107,7 @@ export class ERBNoExtraWhitespaceRule extends ParserRule<ERBNoExtraWhitespaceAut
}

check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense<ERBNoExtraWhitespaceAutofixContext>[] {
const visitor = new ERBNoExtraWhitespaceInsideTagsVisitor(this.name, context)
const visitor = new ERBNoExtraWhitespaceInsideTagsVisitor(this.ruleName, context)

visitor.visit(result.value)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class ERBNoInlineCaseConditionsVisitor extends BaseRuleVisitor {
}

export class ERBNoInlineCaseConditionsRule extends ParserRule {
name = "erb-no-inline-case-conditions"
static ruleName = "erb-no-inline-case-conditions"

get defaultConfig(): FullRuleConfig {
return {
Expand All @@ -45,7 +45,7 @@ export class ERBNoInlineCaseConditionsRule extends ParserRule {
}

check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
const visitor = new ERBNoInlineCaseConditionsVisitor(this.name, context)
const visitor = new ERBNoInlineCaseConditionsVisitor(this.ruleName, context)

visitor.visit(result.value)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class ERBNoOutputControlFlowRuleVisitor extends BaseRuleVisitor {
}

export class ERBNoOutputControlFlowRule extends ParserRule {
name = "erb-no-output-control-flow"
static ruleName = "erb-no-output-control-flow"

get defaultConfig(): FullRuleConfig {
return {
Expand All @@ -60,7 +60,7 @@ export class ERBNoOutputControlFlowRule extends ParserRule {
}

check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
const visitor = new ERBNoOutputControlFlowRuleVisitor(this.name, context)
const visitor = new ERBNoOutputControlFlowRuleVisitor(this.ruleName, context)

visitor.visit(result.value)

Expand Down
Loading
Loading