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 shared variable in detekt-extension rules #1897

Merged
merged 2 commits into from
Sep 12, 2019
Merged

Fix shared variable in detekt-extension rules #1897

merged 2 commits into from
Sep 12, 2019

Conversation

schalkms
Copy link
Member

@schalkms schalkms commented Sep 6, 2019

No description provided.

@@ -40,6 +40,7 @@ class TooManyFunctionsTwo(config: Config) : ThresholdRule(config, THRESHOLD) {
references = emptyList())
)
}
amount = 0
Copy link
Member Author

Choose a reason for hiding this comment

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

Do we really need two example rules that basically do the same?

Copy link
Member

@3flex 3flex Sep 10, 2019

Choose a reason for hiding this comment

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

Maybe it's to demonstrate how the rule provider works when registering multiple rules (one with, one without custom config)? But maybe removing the second example and just adding a comment to SampleProvider is enough.

They don't require much maintenance though so we could just leave it.

Copy link
Member

Choose a reason for hiding this comment

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

That exactly was the intention. One can be configured, the other not. One reports ThresholdedCodeSmell other just CodeSmell.

@codecov-io
Copy link

codecov-io commented Sep 6, 2019

Codecov Report

Merging #1897 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1897   +/-   ##
=========================================
  Coverage     80.25%   80.25%           
  Complexity     1949     1949           
=========================================
  Files           325      325           
  Lines          5504     5504           
  Branches       1019     1019           
=========================================
  Hits           4417     4417           
  Misses          557      557           
  Partials        530      530

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 63c1d23...09362d3. Read the comment docs.

@@ -40,6 +40,7 @@ class TooManyFunctionsTwo(config: Config) : ThresholdRule(config, THRESHOLD) {
references = emptyList())
)
}
amount = 0
Copy link
Member

Choose a reason for hiding this comment

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

That exactly was the intention. One can be configured, the other not. One reports ThresholdedCodeSmell other just CodeSmell.

@arturbosch arturbosch merged commit f714f6e into detekt:master Sep 12, 2019
@arturbosch arturbosch added this to the 1.0.2 milestone Sep 12, 2019
sowmyav24 pushed a commit to sowmyav24/detekt that referenced this pull request Sep 17, 2019
* Fix shared variable in detekt-extension rules

* Use KtFile instead of PsiFile visitor
sowmyav24 pushed a commit to sowmyav24/detekt that referenced this pull request Sep 17, 2019
* Fix shared variable in detekt-extension rules

* Use KtFile instead of PsiFile visitor
@arturbosch arturbosch modified the milestones: 1.0.2, 1.1.0 Sep 17, 2019
@schalkms schalkms deleted the detekt-extensions branch November 8, 2019 18:11
smyachenkov pushed a commit to smyachenkov/detekt that referenced this pull request Dec 9, 2019
* Fix shared variable in detekt-extension rules

* Use KtFile instead of PsiFile visitor
smyachenkov pushed a commit to smyachenkov/detekt that referenced this pull request Dec 9, 2019
* Fix shared variable in detekt-extension rules

* Use KtFile instead of PsiFile visitor
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.

4 participants