Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix superfluous disable command for custom rules #5546

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

mildm8nnered
Copy link
Collaborator

@mildm8nnered mildm8nnered commented Apr 27, 2024

Very much a work in progress, so please ignore for now.

This is redo of #4755 (which was reverted in #5336).

I took the original changes from #4755 and worked from there. The changes I've made on top of the original PR can be seen at mildm8nnered/SwiftLint@mildm8nnered-marcelos-changes-back...mildm8nnered:SwiftLint:mildm8nnered-fix-superfluous-disable-command-for-custom-rules

I thought this was good, but custom_rules now seems to be picking up issues that it didn't see before (fatalError), and there were some issues with superfluous triggering in some tests that I was not expecting either, so I think more work is required here.

@SwiftLintBot
Copy link

SwiftLintBot commented Apr 27, 2024

1 Warning
⚠️ If this is a user-facing change, please include a CHANGELOG entry to credit yourself!
You can find it at CHANGELOG.md.
17 Messages
📖 Linting Aerial with this PR took 1.13s vs 1.15s on main (1% faster)
📖 Linting Alamofire with this PR took 1.64s vs 1.63s on main (0% slower)
📖 Linting Brave with this PR took 9.42s vs 9.42s on main (0% slower)
📖 Linting DuckDuckGo with this PR took 4.98s vs 4.95s on main (0% slower)
📖 Linting Firefox with this PR took 12.49s vs 12.42s on main (0% slower)
📖 Linting Kickstarter with this PR took 11.76s vs 11.47s on main (2% slower)
📖 Linting Moya with this PR took 0.65s vs 0.63s on main (3% slower)
📖 Linting NetNewsWire with this PR took 3.4s vs 3.41s on main (0% faster)
📖 Linting Nimble with this PR took 0.94s vs 0.92s on main (2% slower)
📖 Linting PocketCasts with this PR took 9.32s vs 9.36s on main (0% faster)
📖 Linting Quick with this PR took 0.43s vs 0.42s on main (2% slower)
📖 Linting Realm with this PR took 5.75s vs 5.73s on main (0% slower)
📖 Linting Sourcery with this PR took 2.93s vs 2.89s on main (1% slower)
📖 Linting Swift with this PR took 5.74s vs 5.7s on main (0% slower)
📖 Linting VLC with this PR took 1.55s vs 1.52s on main (1% slower)
📖 Linting Wire with this PR took 21.3s vs 20.91s on main (1% slower)
📖 Linting WordPress with this PR took 14.32s vs 14.29s on main (0% slower)

Here's an example of your CHANGELOG entry:

* Fix superfluous disable command for custom rules.  
  [mildm8nnered](https://github.com/mildm8nnered)
  [#issue_number](https://github.com/realm/SwiftLint/issues/issue_number)

note: There are two invisible spaces after the entry's text.

Generated by 🚫 Danger

@mildm8nnered mildm8nnered force-pushed the mildm8nnered-fix-superfluous-disable-command-for-custom-rules branch 4 times, most recently from 2aa98d0 to ceb3969 Compare May 4, 2024 16:53
@mildm8nnered mildm8nnered force-pushed the mildm8nnered-fix-superfluous-disable-command-for-custom-rules branch from ceb3969 to 344f376 Compare May 11, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants