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 AbstractTypeAwareCheck when dealing with nested interfaces #3835

Merged
merged 1 commit into from Feb 16, 2017

Conversation

Projects
None yet
3 participants
@rnveach
Member

rnveach commented Feb 16, 2017

AbstractTypeAwareCheck looks like it could contain a bug when dealing with interfaces, especially nested interfaces.

When we process visitToken, we call processClass for classes, interfaces, and enumerations.
processClass manipulates currentClassName and calls processTypeParams to handle the type parameters.
When we process leaveToken for these tokens we do the same as processClass but reversing it, except we left out interfaces.

This fix just adds interface to the tokens we do this action on. Without it, we won't have correct internal variables in nested interfaces after leaving the interface.

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Feb 16, 2017

Codecov Report

Merging #3835 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #3835   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         275     275           
  Lines       13635   13636    +1     
  Branches     3069    3070    +1     
======================================
+ Hits        13635   13636    +1
Impacted Files Coverage Δ
...ools/checkstyle/checks/AbstractTypeAwareCheck.java 100% <100%> (ø)

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 34ef127...026ad80. Read the comment docs.

codecov-io commented Feb 16, 2017

Codecov Report

Merging #3835 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #3835   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         275     275           
  Lines       13635   13636    +1     
  Branches     3069    3070    +1     
======================================
+ Hits        13635   13636    +1
Impacted Files Coverage Δ
...ools/checkstyle/checks/AbstractTypeAwareCheck.java 100% <100%> (ø)

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 34ef127...026ad80. Read the comment docs.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Feb 16, 2017

Member

this is not a minor.
It is functional change, it might resolve few bugs :) that we already have registered but do not consider as whole functionality is deprecated but still function.

with minor prefix it will not be referenced even in release notes.

please update commit message prefix to reference this PR number and update PR description to be ready to reused in release notes.

Member

romani commented Feb 16, 2017

this is not a minor.
It is functional change, it might resolve few bugs :) that we already have registered but do not consider as whole functionality is deprecated but still function.

with minor prefix it will not be referenced even in release notes.

please update commit message prefix to reference this PR number and update PR description to be ready to reused in release notes.

@romani romani closed this Feb 16, 2017

@romani romani reopened this Feb 16, 2017

@rnveach rnveach changed the title from minor: added missing leave token to AbstractTypeAwareCheck to Fix AbstractTypeAwareCheck when dealing with nested interfaces Feb 16, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Feb 16, 2017

Member

please update commit message prefix to reference this PR number and update PR description to be ready to reused in release notes.

Done.

Member

rnveach commented Feb 16, 2017

please update commit message prefix to reference this PR number and update PR description to be ready to reused in release notes.

Done.

@romani romani merged commit 48d48fd into checkstyle:master Feb 16, 2017

8 checks passed

IDEA Inspections Pull Request (Checkstyle) TeamCity build finished
Details
Shippable Run 979 status is SUCCESS.
Details
codacy/pr Good work! A positive pull request.
Details
codecov/patch 100% of diff hit (target 100%)
Details
codecov/project 100% (+0%) compared to 34ef127
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
wercker/build Wercker pipeline passed
Details

@rnveach rnveach deleted the rnveach:abstract_type_minor branch Feb 16, 2017

@rnveach rnveach added this to the 7.6 milestone Feb 23, 2017

@romani romani added bug approved and removed bug labels Feb 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment