Skip to content

Commit a2a31d4

Browse files
authored
Linter CLI: Improve --upgrade to only disable rules with offenses (#1484)
This pull request is a follow up on #1453 and improves the `--upgrade` command in the linter CLI. Previously, `herb-lint --upgrade` blindly disabled all newly introduced rules. Now it lints the codebase first with the new rules and: - Enables rules that produce zero offenses (safe to adopt immediately) - Disables only rules that have existing offenses, with offense counts shown <img width="2532" height="1898" alt="CleanShot 2026-03-23 at 23 49 16@2x" src="https://github.com/user-attachments/assets/6a1e8f39-5059-4788-ab64-f73c881db124" /> Demo: https://github.com/user-attachments/assets/931754bf-a3be-4c9c-a97d-ebf2578fde24
1 parent 53be911 commit a2a31d4

File tree

6 files changed

+240
-280
lines changed

6 files changed

+240
-280
lines changed

javascript/packages/linter/src/cli.ts

Lines changed: 73 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -192,15 +192,58 @@ export class CLI {
192192

193193
const { skippedByVersion } = Linter.filterRulesByConfig(rules, config.linter?.rules, configVersion)
194194

195-
const rulesMutation: Record<string, { enabled: boolean }> = {}
195+
let rulesToDisable: typeof skippedByVersion = []
196+
let rulesToEnable: typeof skippedByVersion = []
197+
const ruleOffenseCounts = new Map<string, number>()
196198

197-
for (const rule of skippedByVersion) {
198-
rulesMutation[rule.ruleName] = { enabled: false }
199-
}
199+
if (skippedByVersion.length > 0) {
200+
console.log(`\n${colorize("↻", "cyan")} Checking ${colorize(String(skippedByVersion.length), "bold")} new ${skippedByVersion.length === 1 ? "rule" : "rules"} against your codebase...`)
201+
202+
const skippedRulesConfig: Record<string, { enabled: boolean }> = {}
200203

201-
await Config.mutateConfigFile(config.path, {
202-
linter: { rules: rulesMutation }
203-
})
204+
for (const rule of skippedByVersion) {
205+
skippedRulesConfig[rule.ruleName] = { enabled: true }
206+
}
207+
208+
const upgradeConfig = Config.fromObject({
209+
...config.options,
210+
linter: {
211+
...config.options.linter,
212+
rules: { ...config.options.linter?.rules, ...skippedRulesConfig }
213+
}
214+
}, { projectPath: this.projectPath, configVersion: version })
215+
216+
const upgradeContext: ProcessingContext = {
217+
projectPath: this.projectPath,
218+
config: upgradeConfig,
219+
jobs,
220+
}
221+
222+
await Herb.load()
223+
224+
const files = await config.findFilesForTool('linter', this.projectPath)
225+
const upgradeProcessor = new FileProcessor()
226+
const results = await upgradeProcessor.processFiles(files, 'json', upgradeContext)
227+
228+
for (const [ruleName, data] of results.ruleOffenses) {
229+
ruleOffenseCounts.set(ruleName, data.count)
230+
}
231+
232+
rulesToDisable = skippedByVersion.filter(rule => ruleOffenseCounts.has(rule.ruleName))
233+
rulesToEnable = skippedByVersion.filter(rule => !ruleOffenseCounts.has(rule.ruleName))
234+
235+
const rulesMutation: Record<string, { enabled: boolean }> = {}
236+
237+
for (const rule of rulesToDisable) {
238+
rulesMutation[rule.ruleName] = { enabled: false }
239+
}
240+
241+
if (Object.keys(rulesMutation).length > 0) {
242+
await Config.mutateConfigFile(config.path, {
243+
linter: { rules: rulesMutation }
244+
})
245+
}
246+
}
204247

205248
const { promises: fs } = await import("fs")
206249
let content = await fs.readFile(config.path, "utf-8")
@@ -209,16 +252,29 @@ export class CLI {
209252

210253
console.log(`\n${colorize("✓", "brightGreen")} Updated ${colorize(".herb.yml", "cyan")} version from ${colorize(configVersion, "cyan")} to ${colorize(version, "cyan")}`)
211254

212-
if (skippedByVersion.length > 0) {
213-
console.log(`${colorize("✓", "brightGreen")} Disabled ${colorize(String(skippedByVersion.length), "bold")} newly introduced ${skippedByVersion.length === 1 ? "rule" : "rules"}:`)
214-
console.log("")
255+
if (rulesToEnable.length > 0) {
256+
console.log(`\n${colorize("✓", "brightGreen")} Enabled ${colorize(String(rulesToEnable.length), "bold")} new ${rulesToEnable.length === 1 ? "rule" : "rules"} (no offenses found):\n`)
215257

216-
for (const rule of skippedByVersion) {
217-
console.log(` ${colorize(rule.ruleName, "white")}: ${colorize("enabled: false", "gray")}`)
258+
for (const rule of rulesToEnable) {
259+
console.log(` ${colorize("✓", "brightGreen")} ${colorize(rule.ruleName, "white")}`)
260+
}
261+
}
262+
263+
if (rulesToDisable.length > 0) {
264+
const totalOffenses = Array.from(ruleOffenseCounts.values()).reduce((sum, count) => sum + count, 0)
265+
266+
console.log(`\n${colorize("!", "yellow")} Found ${colorize(String(totalOffenses), "bold")} ${totalOffenses === 1 ? "offense" : "offenses"} across ${colorize(String(rulesToDisable.length), "bold")} new ${rulesToDisable.length === 1 ? "rule" : "rules"}. Disabled to ease the upgrade:\n`)
267+
268+
for (const rule of rulesToDisable) {
269+
const offenseCount = ruleOffenseCounts.get(rule.ruleName) || 0
270+
console.log(` ${colorize("✗", "red")} ${colorize(rule.ruleName, "white")} ${colorize(`(${offenseCount} ${offenseCount === 1 ? "offense" : "offenses"})`, "gray")}`)
218271
}
219272

220-
console.log("")
221-
console.log(` Enable rules individually in your ${colorize(".herb.yml", "cyan")} when you're ready.`)
273+
console.log(`\n When you're ready, review the disabled ${rulesToDisable.length === 1 ? "rule" : "rules"} in your ${colorize(".herb.yml", "cyan")} and re-enable them after fixing the offenses.`)
274+
}
275+
276+
if (skippedByVersion.length === 0) {
277+
console.log(`\n${colorize("✓", "brightGreen")} No new rules to configure.`)
222278
}
223279

224280
console.log("")
@@ -299,6 +355,8 @@ export class CLI {
299355
processingConfig = modifiedConfig
300356
}
301357

358+
const hasConfigFile = Config.exists(configFile || this.projectPath)
359+
302360
const context: ProcessingContext = {
303361
projectPath: this.projectPath,
304362
configPath: configFile,
@@ -308,6 +366,7 @@ export class CLI {
308366
ignoreDisableComments,
309367
linterConfig,
310368
config: processingConfig,
369+
hasConfigFile,
311370
loadCustomRules,
312371
jobs
313372
}

javascript/packages/linter/src/cli/file-processor.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export interface ProcessingContext {
3333
ignoreDisableComments?: boolean
3434
linterConfig?: HerbConfigOptions['linter']
3535
config?: Config
36+
hasConfigFile?: boolean
3637
loadCustomRules?: boolean
3738
jobs?: number
3839
}

javascript/packages/linter/src/cli/output-manager.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ export class OutputManager {
6262
ignoreDisableComments: context?.ignoreDisableComments,
6363
rulesSkippedByVersion,
6464
configVersion: context?.config?.configVersion,
65+
configPath: context?.config?.path,
66+
hasConfigFile: context?.hasConfigFile,
6567
toolVersion: options.toolVersion,
6668
})
6769
}
@@ -125,6 +127,8 @@ export class OutputManager {
125127
ignoreDisableComments: context?.ignoreDisableComments,
126128
rulesSkippedByVersion,
127129
configVersion: context?.config?.configVersion,
130+
configPath: context?.config?.path,
131+
hasConfigFile: context?.hasConfigFile,
128132
toolVersion: options.toolVersion,
129133
})
130134
}

javascript/packages/linter/src/cli/summary-reporter.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ export interface SummaryData {
2323
ignoreDisableComments?: boolean
2424
rulesSkippedByVersion?: VersionSkippedRule[]
2525
configVersion?: string
26+
configPath?: string
27+
hasConfigFile?: boolean
2628
toolVersion?: string
2729
}
2830

@@ -132,22 +134,24 @@ export class SummaryReporter {
132134
console.log(` ${colorize("✓", "brightGreen")} ${colorize("All files are clean!", "green")}`)
133135
}
134136

135-
this.displayVersionSkippedRules(data.rulesSkippedByVersion, data.configVersion, data.toolVersion)
137+
this.displayVersionSkippedRules(data)
136138
}
137139

138-
displayVersionSkippedRules(skippedRules?: VersionSkippedRule[], configVersion?: string, toolVersion?: string): void {
140+
displayVersionSkippedRules(data: SummaryData): void {
141+
const { rulesSkippedByVersion: skippedRules, configVersion, configPath, hasConfigFile, toolVersion } = data
142+
139143
if (!skippedRules || skippedRules.length === 0) return
144+
if (!hasConfigFile) return
140145

141146
const ruleCount = skippedRules.length
142147
const suggestedVersion = toolVersion || configVersion || "latest"
143148

144149
console.log("")
145150
console.log(` ${colorize(`New rules available:`, "bold")}`)
151+
console.log(` Your ${colorize(".herb.yml", "cyan")} version is ${colorize(configVersion!, "cyan")}. ${colorize(String(ruleCount), "bold")} new ${this.pluralize(ruleCount, "rule")} ${ruleCount === 1 ? "is" : "are"} disabled to ease upgrades:`)
146152

147-
if (configVersion) {
148-
console.log(` Your ${colorize(".herb.yml", "cyan")} version is ${colorize(configVersion, "cyan")}. ${colorize(String(ruleCount), "bold")} new ${this.pluralize(ruleCount, "rule")} ${ruleCount === 1 ? "is" : "are"} disabled to ease upgrades:`)
149-
} else {
150-
console.log(` ${colorize(String(ruleCount), "bold")} ${this.pluralize(ruleCount, "rule")} ${ruleCount === 1 ? "is" : "are"} available in newer versions:`)
153+
if (configPath) {
154+
console.log(` ${colorize("from Herb config:", "gray")} ${colorize(configPath, "cyan")}`)
151155
}
152156

153157
console.log("")
@@ -174,8 +178,8 @@ export class SummaryReporter {
174178
}
175179

176180
console.log("")
177-
console.log(` Run ${colorize("herb-lint --upgrade", "cyan")} to update the version and disable all new rules, or`)
178-
console.log(` update the version in your ${colorize(".herb.yml", "cyan")} to ${colorize(`"${suggestedVersion}"`, "cyan")} to enable them all at once.`)
181+
console.log(` Run ${colorize("herb-lint --upgrade", "cyan")} to update the version. Rules with no offenses will be`)
182+
console.log(` enabled automatically; rules with offenses will be disabled to ease the upgrade.`)
179183
}
180184

181185
displayMostViolatedRules(ruleOffenses: Map<string, { count: number, files: Set<string> }>, limit: number = 5): void {

0 commit comments

Comments
 (0)