realm / SwiftLint

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

Print only file path when reporting a correction #5596

Closed SimplyDanny closed 6 days ago

SimplyDanny commented 1 month ago

Omit exact line and column information. Keep the internal logic available for a while in case people complain for good reasons. Then the change can be reverted easily.

Reasons behind this change:

SwiftLintBot commented 1 month ago
17 Messages
:book: Linting Aerial with this PR took 0.8s vs 0.81s on main (1% faster)
:book: Linting Alamofire with this PR took 1.13s vs 1.14s on main (0% faster)
:book: Linting Brave with this PR took 6.66s vs 6.62s on main (0% slower)
:book: Linting DuckDuckGo with this PR took 3.51s vs 3.49s on main (0% slower)
:book: Linting Firefox with this PR took 9.79s vs 9.86s on main (0% faster)
:book: Linting Kickstarter with this PR took 8.21s vs 8.16s on main (0% slower)
:book: Linting Moya with this PR took 0.49s vs 0.48s on main (2% slower)
:book: Linting NetNewsWire with this PR took 2.26s vs 2.24s on main (0% slower)
:book: Linting Nimble with this PR took 0.68s vs 0.68s on main (0% slower)
:book: Linting PocketCasts with this PR took 7.05s vs 7.11s on main (0% faster)
:book: Linting Quick with this PR took 0.38s vs 0.37s on main (2% slower)
:book: Linting Realm with this PR took 4.32s vs 4.3s on main (0% slower)
:book: Linting Sourcery with this PR took 2.08s vs 2.07s on main (0% slower)
:book: Linting Swift with this PR took 4.02s vs 4.01s on main (0% slower)
:book: Linting VLC with this PR took 1.13s vs 1.11s on main (1% slower)
:book: Linting Wire with this PR took 15.12s vs 15.06s on main (0% slower)
:book: Linting WordPress with this PR took 11.59s vs 11.53s on main (0% slower)

Generated by :no_entry_sign: Danger

mildm8nnered commented 1 week ago

I think if the information we're providing now is misleading, then suppressing it is ok.

The list of problems above makes me think that maybe we're doing something wrong here - they all sound fixable - at the end of the day it's just a set of transformations on a set of files.

My personal use-case for autocorrect is that we run it during every build for whitespace correction, colon spacing etc, so no-one ever looks at the actual violation positions, and we never enable autocorrect for new rules - we just fix them manually, or add them to our baseline - not for any principled reason - we just never have.

I guess the other use case is having a legacy code-base and doing a huge cleanup sweep - in theory that should work, but personally I'd be a little bit wary of turning on fix for everything and letting rip.

If I did care about fix violation positions, and wanted to see them in Xcode for example, I think we could come up with accurate violation positions, taking into account all the other fixes, but we would need to take into account all the other fixes, and that sounds like a lot of work for something that I'm not sure many people would ever look at.

SimplyDanny commented 6 days ago

Yes, there are potential ways to fix this and get it right. Yet, they don't seem to be easy. We will see if anyone is relying on these positions. I actually don't think so given that these locations are just wrong most of the time. For the moment, it's better to have a simpler output that is correct.