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 reporting unused imports at file level #6390

Merged
merged 1 commit into from Aug 30, 2023

Conversation

arturbosch
Copy link
Member

@arturbosch arturbosch commented Aug 14, 2023

This PR was triggered by users of the IntelliJ plugin getting every unused import reported at the file level which clutters the editor: detekt/detekt-intellij-plugin#486 (comment)

Screenshot from 2023-08-14 19-57-14

This PR partly reverts the change to the emit logic of https://github.com/detekt/detekt/pull/5876/files#diff-9cf77f9ffcbf036bf5044d1ef99728b56aad97854246f3473f69d54b4b929380.
Emiting at the node is wrong in the case of the NoUnusedImports rule as in beforeVisitChildNodes the import usages are calculated and only reported in afterVisitChildNodes from the file node (because fileNode.imports access is easy I suppose).

With this revert the lines are again calculated with the offset function and lead to the same results as our MaxLineLength rule:
Screenshot from 2023-08-14 20-16-34

The other changed test cases are also now correct again e,g, the tested code for indentation rule is
val code = "fun main() {\n println()\n}" so .hasStartSourceLocation(1, 13) could not be correct due to the line breaks.

Fixes #6105

Copy link
Member

@cortinico cortinico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this @arturbosch
Do we want to backport this to 1.x?

@arturbosch
Copy link
Member Author

Thanks for fixing this @arturbosch Do we want to backport this to 1.x?

Yes please, this is a regression we introduced in 1.23.0 so I suggest a fix in 1.23.2 :)

@cortinico cortinico added the pick request Marker for PRs that should be ported to the 1.0 release branch label Aug 21, 2023
@cortinico cortinico added this to the 2.0.0 milestone Aug 21, 2023
@BraisGabin BraisGabin merged commit cd486e4 into main Aug 30, 2023
21 checks passed
@BraisGabin BraisGabin deleted the fix-nounused-imports-reporting branch August 30, 2023 12:20
3flex added a commit to 3flex/detekt that referenced this pull request Aug 31, 2023
Caused by detekt#6390 being merged after detekt#6397 without a rebase first
@3flex 3flex mentioned this pull request Aug 31, 2023
3flex added a commit that referenced this pull request Aug 31, 2023
Caused by #6390 being merged after #6397 without a rebase first
mgroth0 pushed a commit to mgroth0/detekt that referenced this pull request Feb 11, 2024
mgroth0 pushed a commit to mgroth0/detekt that referenced this pull request Feb 11, 2024
Caused by detekt#6390 being merged after detekt#6397 without a rebase first
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatting pick request Marker for PRs that should be ported to the 1.0 release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ktlint violation location in detekt-formatting is broader
5 participants