realm / SwiftLint

A tool to enforce Swift style and conventions.
https://realm.github.io/SwiftLint
MIT License
18.59k stars 2.22k forks source link

superfluous_disable_command bug #5788

Open artsimonyan23 opened 6 days ago

artsimonyan23 commented 6 days ago

superfluous_disable_command incorrectly runs all custom rules.

SimplyDanny commented 6 days ago

Please provide a concrete example.

artsimonyan23 commented 6 days ago

I created a custom rule weak_self_closure. In some places, I disabled it ( // swiftlint:disable:this weak_self_closure). After updating with the new version (0.57.0) in all disabled places I receive superfluous_disable_command warning. When I am removing the "// swiftlint:disable:this weak_self_closure", it shows "weak_self_closure" warning.

mildm8nnered commented 3 days ago

I wish I'd never touched this.

I think there is a problem here, where disabled regions are nested.

Given the following config:

only_rules:
  - custom_rules
  - superfluous_disable_command

# Custom rules
custom_rules:
  fatal_error:
    name: Fatal Error
    excluded: "Tests/*"
    message: Prefer using `queuedFatalError` over `fatalError` to avoid leaking compiler host machine paths.
    regex: \bfatalError\b
    match_kinds:
      - identifier
    severity: error
  fattal_error:
    name: Fattal Error
    excluded: "Tests/*"
    message: Prefer using `queuedFatalError` over `fatalError` to avoid leaking compiler host machine paths.
    regex: \bfattalError\b
    match_kinds:
      - identifier
    severity: error    

and the following input:

// swiftlint:disable fatal_error
// swiftlint:disable:next fattal_error
fatalError("something")
// swiftlint:enable fatal_error

then

% swiftlint tt.swift --config tt.config
Linting Swift files at paths tt.swift
Linting 'tt.swift' (1/1)
/Users/martin.redington/Documents/Source/SwiftLint/tt.swift:1:33: warning: Superfluous Disable Command Violation: SwiftLint rule 'fatal_error' did not trigger a violation in the disabled region; remove the disable command (superfluous_disable_command)
/Users/martin.redington/Documents/Source/SwiftLint/tt.swift:3:1: warning: Superfluous Disable Command Violation: SwiftLint rule 'fattal_error' did not trigger a violation in the disabled region; remove the disable command (superfluous_disable_command)
/Users/martin.redington/Documents/Source/SwiftLint/tt.swift:3:9223372036854775807: warning: Superfluous Disable Command Violation: SwiftLint rule 'fatal_error' did not trigger a violation in the disabled region; remove the disable command (superfluous_disable_command)
Done linting! Found 3 violations, 0 serious in 1 file.

But fatalError should not be triggering superfluous_disable_command here at all.

I think this is being caused by the way regions are returned from SwiftLintFile's regions(restrictingRuleIdentifiers:) method, and I'm hoping we can fix it just by making a change there - right now each new swiftlint: command seems to generate a region with all disabled identifiers, but we probably really want a region for each command.