-
Notifications
You must be signed in to change notification settings - Fork 314
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
Detekt: Add auto-correct functionality for OrtImportOrder
#9768
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9768 +/- ##
=========================================
Coverage 68.07% 68.07%
+ Complexity 1285 1284 -1
=========================================
Files 249 249
Lines 8827 8827
Branches 918 918
=========================================
Hits 6009 6009
Misses 2432 2432
Partials 386 386
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Could you also update https://github.com/oss-review-toolkit/ort/blob/main/website/docs/development.md#detekt?
7d5a4b1
to
8a08215
Compare
@MarcelBochtler This is really great. Does the auto-correct also remove unused imports ? |
We use Detekt's We would therefore need to switch to the Lines 23 to 24 in 7f8d3d3
But I did not research if this reasoning is still valid. |
@@ -68,7 +68,7 @@ class OrtImportOrder(config: Config) : Rule(config) { | |||
if (importPaths != expectedImportOrder) { | |||
val path = importList.containingKtFile.absolutePath() | |||
val message = "Imports in file '$path' are not sorted alphabetically or single blank lines are missing " + | |||
"between different top-level packages" | |||
"between different top-level packages." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, while I'm a known fan of properly ending sentences with dots, in this case the dot was deliberately omitted to align with built-in detekt rules, which suffix the message with the name of the rule in square brackets, e.g. Needless blank line(s) [NoConsecutiveBlankLines]
.
So I'd have a slight preference for dropping this commit. But I'm also ok to keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropped the commit, I agree that this message should be in line with other detekt messages.
I believe this comment is still open, @MarcelBochtler. An oversight? |
Add the functionality to use Detekt's auto-correct functionality [1] to fix an incorrect import order or missing empty lines between top level domains. To use this, run the `detekt` Gradle task with the `--auto-correct` flag: ``` ./gradlew detekt --auto-correct ``` [1]: https://detekt.dev/docs/introduction/extensions/#autocorrect-property Signed-off-by: Marcel Bochtler <[email protected]>
Signed-off-by: Marcel Bochtler <[email protected]>
8a08215
to
48c361c
Compare
I was aware of it ;) I also dropped the information about a pre-commit hook from the commit message. |
Add the functionality to use Detekt's auto-correct functionality 1 to
fix an incorrect import order or missing empty lines between top level
domains.
To use this, run the
detekt
Gradle task with the--auto-correct
flag:
auto-correct
can also be useful as a Git pre-commit hook using thedetekt-cli 2: