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

Added classPath argument to detekt plugin, fixed detected issues #238

Merged
merged 32 commits into from
Sep 17, 2020

Conversation

petertrr
Copy link
Member

@petertrr petertrr commented Sep 7, 2020

Additional checks involving type resolution are activated if detekt is aware of classpath. It then involves kotlin compiler to create more complex stuff than just AST. This configuration discovered couple of dozens of unsafe !! in our code.

Fixme:

  • there are currently a lot of errors like unresolved reference in build logs, though everything seems to be working fine. - solved with detekt version update (?)
  • the only limitation left is that detekt doesn't split classpath correctly on windows (warning: classpath entry points to a non-existent location: C when path starts with C:\), but nothing can be done on our side

### What's done:
* Edited pom.xml
* Edited detekt.yml
### What's done:
* Fixed detected code issues
### What's done:
* Added compilation to detekt workflow
@codecov
Copy link

codecov bot commented Sep 7, 2020

Codecov Report

Merging #238 into master will increase coverage by 0.07%.
The diff coverage is 61.03%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #238      +/-   ##
============================================
+ Coverage     81.25%   81.33%   +0.07%     
- Complexity      957      959       +2     
============================================
  Files            50       50              
  Lines          2524     2524              
  Branches        788      789       +1     
============================================
+ Hits           2051     2053       +2     
+ Misses          190      187       -3     
- Partials        283      284       +1     
Flag Coverage Δ Complexity Δ
#unittests 81.33% <61.03%> (+0.07%) 959.00 <3.00> (+2.00)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
.../cqfn/diktat/ruleset/rules/BlockStructureBraces.kt 82.65% <0.00%> (-1.03%) 53.00 <0.00> (ø)
...kotlin/org/cqfn/diktat/ruleset/rules/EmptyBlock.kt 66.66% <ø> (ø) 10.00 <0.00> (ø)
...in/org/cqfn/diktat/ruleset/rules/EnumsSeparated.kt 91.42% <ø> (ø) 16.00 <0.00> (ø)
...kotlin/org/cqfn/diktat/ruleset/rules/FileNaming.kt 9.52% <0.00%> (+0.82%) 2.00 <0.00> (ø)
...kotlin/org/cqfn/diktat/ruleset/rules/LineLength.kt 87.12% <ø> (ø) 74.00 <0.00> (ø)
...in/org/cqfn/diktat/ruleset/rules/WhiteSpaceRule.kt 79.67% <ø> (ø) 110.00 <0.00> (ø)
.../cqfn/diktat/ruleset/rules/files/BlankLinesRule.kt 100.00% <ø> (ø) 12.00 <0.00> (ø)
...rg/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt 87.90% <ø> (ø) 75.00 <0.00> (ø)
...at/ruleset/rules/identifiers/LocalVariablesRule.kt 92.06% <ø> (ø) 32.00 <0.00> (ø)
.../kotlin/org/cqfn/diktat/ruleset/utils/KDocUtils.kt 6.66% <ø> (ø) 0.00 <0.00> (ø)
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b59bdfe...240b05c. Read the comment docs.

…detekt-plugin

# Conflicts:
#	diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/FileNaming.kt
### What's done:
* Override detekt dependency
### What's done:
* Corrected errors
### What's done:
* Corrected errors
### What's done:
* Corrected errors
### What's done:
* Specify java and kotlin versions in config
### What's done:
* Specify java and kotlin versions in config
### What's done:
* Fixing issues
### What's done:
* Fixing issues
### What's done:
* Fixing issues
### What's done:
* Fixing issues
…detekt-plugin

# Conflicts:
#	diktat-common/src/test/kotlin/org/cqfn/diktat/test/ConfigReaderTest.kt
#	diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/kdoc/KdocFormatting.kt
#	pom.xml
…detekt-plugin

# Conflicts:
#	diktat-common/src/test/kotlin/org/cqfn/diktat/test/ConfigReaderTest.kt
#	diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/kdoc/KdocFormatting.kt
#	pom.xml
### What's done:
* Cleanup
…' into infra/classpath-in-detekt-plugin

# Conflicts:
#	diktat-common/src/test/kotlin/org/cqfn/diktat/test/ConfigReaderTest.kt
…detekt-plugin

# Conflicts:
#	diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/FileNaming.kt
#	diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt
#	diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt
#	pom.xml
### What's done:
* Fixing discovered issues
### What's done:
* Batch mode for maven in CI workflows
### What's done:
* Fixing discovered issues
### What's done:
* Fixing discovered issues
### What's done:
* Fixing discovered issues
### What's done:
* Fixing discovered issues
### What's done:
* Fixing discovered issues
### What's done:
* Removed override for detekt-cli
@petertrr petertrr marked this pull request as ready for review September 17, 2020 07:42
@petertrr petertrr requested a review from orchestr7 September 17, 2020 07:42
Copy link
Member

@orchestr7 orchestr7 left a comment

Choose a reason for hiding this comment

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

lgtm

### What's done:
* Revert changes in PackageNaming.kt
@petertrr petertrr merged commit 255d60f into master Sep 17, 2020
@petertrr petertrr deleted the infra/classpath-in-detekt-plugin branch September 17, 2020 10:21
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.

2 participants