Add auto-correction, and multi-violation support to multiline_call_arguments - #6745
Conversation
b183a6a to
78a2da1
Compare
|
@SimplyDanny Let's add correctable for The CI failure on oss_scan is not related to this PR — it's a danger bot failing to post a comment due to the repository's interaction limits (422 - Interactions on this repository have been restricted to prior contributors only) |
f16bd4c to
8b5b043
Compare
Generated by 🚫 Danger |
3c09c9b to
c2b5aa8
Compare
9510ef7 to
0cdc2a7
Compare
7b65161 to
9a91acd
Compare
|
@SimplyDanny It would be great if you could review the PR :) |
9a91acd to
0bd454f
Compare
ebc98ef to
f694a41
Compare
8b03add to
1d6eaeb
Compare
SimplyDanny
left a comment
There was a problem hiding this comment.
A few more comments ...
In general, I'm not against using AI to help with coding. However, I expect from contributors that they understand what they are proposing, review the code and clean things up themselves.
I see a lot of redundancy and useless information in comments as well as unused code that I shouldn't not need to take care of as a reviewer. There are many other PRs waiting for my feedback. So please make sure to propose only changes that you have thoroughly reviewed and improved yourself. Otherwise, I might lose my motivation to review anything like this. AI only has a chance to be of help if it makes use faster as a whole. Shifting work from you to someone else doesn't increase throughput at all.
I completely agree with you about speeding up work with AI without shifting. |
I appreciate you running such extensive tests. The implementation and coverage look decent indeed. However, "it works" is not all I have an eye on. Project fit, simplicity and avoidance of redundancy are of similar importance. There is also nothing against letting AI document things. But sometimes no comment is just the right amount, especially in obvious cases. 😉 Thank you for relentlessly fixing all my (nitpicky) comments and the ongoing contributions! |
8b52ebd to
85e828d
Compare
fa9b77a to
e58072b
Compare
|
@SimplyDanny It would be great if you could review it again :) |
462ed9f to
53f3cf7
Compare
c765971 to
152c5bb
Compare
…to multiline_call_arguments # Conflicts: # Source/SwiftLintBuiltInRules/Rules/Lint/MultilineCallArgumentsRuleExamples.swift
ed3ce73 to
5af9811
Compare
5af9811 to
3cd04ac
Compare
| let file = SwiftLintFile(contents: contents) | ||
| // Correct to convergence: no pass may drop an argument. | ||
| var remaining = 5 | ||
| while remaining > 0, rule.correct(file: file) > 0 { |
There was a problem hiding this comment.
Does this mean, not all corrections are performed at once? In other words, would user need to run swiftlint lint --fix multiple times to get all violations corrected?
There was a problem hiding this comment.
All corrections with non-overlapping ranges are applied in a single pass. The loop was a leftover from before multi-violation support. Simplified the test. Multiple passes remain necessary only for nested single-line calls with overlapping ranges.
e44bc71 to
1b61aa7
Compare
4233904 to
6a831e0
Compare
6a831e0 to
ffef965
Compare
|
Thanks for all the work, @GandaLF2006! Did you check if the reported OSS findings are valid? |
|
@SimplyDanny I checked about 60 of the reported OSS findings and they were all valid. |
Summary
Enhances the multiline_call_arguments opt-in rule with auto-correction and multi-violation detection in a single pass.
Auto-correction (
--fix)Multi-violation support
duplicateArgumentStartLineViolationandnewlineAfterCommaViolationreturned at the first match, requiring repeated--fixpasses for calls with multiple same-line argument pairsreasonedViolationsreturns[ReasonedRuleViolation])Documentation
RuleDescription.descriptioninto concisedescription(what) + structuredrationale(why/how) with markdown sections