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

idea: eights part of idea violations #4711

Closed
Nimfadora opened this Issue Jul 13, 2017 · 4 comments

Comments

Projects
2 participants
@Nimfadora
Contributor

Nimfadora commented Jul 13, 2017

sub-task of #4681

too widespread to be included to the report:
Chained method calls 1679
Constant on right side of comparison 1880
Instance field access not qualified with this 3512
Instance method call not qualified with this 11547
Local variable or parameter can be final 3834
Nested method call 6207
Unqualified inner class access 754
Unqualified static access 4421

report on all other violations

violations names included:

ChainedMethodCall
ConstantOnRHSOfComparison
ImplicitCallToSuper
LocalCanBeFinal
NestedMethodCall
UnqualifiedFieldAccess
UnqualifiedInnerClassAccess
UnqualifiedMethodAccess
UnqualifiedStaticUsage
ReturnThis
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 13, 2017

Member

Implicit call to 'super()'

Suppress : "we do not like this style, but we could change our mind in future"

Return of 'this' inspection

All cases are false positives. If there are options in inspections to avoid them, lets suppress "all cases are false positives"

Local variable or parameter can be final

We do not like for final on parameters, but do use our own Check for local variables, please try to focus this inspections on variables only, if no options - suppress

Instance field access not qualified with this 3512
Instance method call not qualified with this 11547

If no options to calm down them suppress. We have our own Check(Require this) on this please look at our Check options to see what we think code should be.

Chained method calls

suppress: "it is too demanding, with no choice limits of chain size or ...."

Constant on right side of comparison

suppress: "we do like constants on the right side, is more readable, placing constant on the left does not give benefit"

Unqualified inner class access

suppress: "it is ok to skip name of outter class for inner class in filed/variable declaration to keep type short."

Unqualified static access

suppress: "inspection does not have option to skip cases of usage static field/method that is declared in the same class, as result there are a lot of want-fix cases."

Member

romani commented Jul 13, 2017

Implicit call to 'super()'

Suppress : "we do not like this style, but we could change our mind in future"

Return of 'this' inspection

All cases are false positives. If there are options in inspections to avoid them, lets suppress "all cases are false positives"

Local variable or parameter can be final

We do not like for final on parameters, but do use our own Check for local variables, please try to focus this inspections on variables only, if no options - suppress

Instance field access not qualified with this 3512
Instance method call not qualified with this 11547

If no options to calm down them suppress. We have our own Check(Require this) on this please look at our Check options to see what we think code should be.

Chained method calls

suppress: "it is too demanding, with no choice limits of chain size or ...."

Constant on right side of comparison

suppress: "we do like constants on the right side, is more readable, placing constant on the left does not give benefit"

Unqualified inner class access

suppress: "it is ok to skip name of outter class for inner class in filed/variable declaration to keep type short."

Unqualified static access

suppress: "inspection does not have option to skip cases of usage static field/method that is declared in the same class, as result there are a lot of want-fix cases."

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Jul 15, 2017

romani added a commit that referenced this issue Jul 15, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 15, 2017

Member

Violations included:

please activate that list of inspections and send PR(not for merge) to generate report on TC.
After each merge to our master, update that PR with list of item that still unresolved, TC will keep report for you.

Member

romani commented Jul 15, 2017

Violations included:

please activate that list of inspections and send PR(not for merge) to generate report on TC.
After each merge to our master, update that PR with list of item that still unresolved, TC will keep report for you.

@romani romani added this to In Progress in Practice What You Preach Jul 15, 2017

@Nimfadora Nimfadora changed the title from idea: code style violations to idea: eights part of idea violations Jul 16, 2017

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Jul 16, 2017

Contributor

@romani this task is also done, these violations were not even included in PR with all remained violations.

Contributor

Nimfadora commented Jul 16, 2017

@romani this task is also done, these violations were not even included in PR with all remained violations.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 16, 2017

Member

ok, am closing this issue as you confirming that all are covered.
We will recheck what is not covered at end of unmbrella issue (#4681).

Member

romani commented Jul 16, 2017

ok, am closing this issue as you confirming that all are covered.
We will recheck what is not covered at end of unmbrella issue (#4681).

@romani romani closed this Jul 16, 2017

@romani romani moved this from In Progress to Done in Practice What You Preach Jul 17, 2017

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