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: seventh part of idea violations #4722

Closed
Nimfadora opened this Issue Jul 15, 2017 · 8 comments

Comments

Projects
2 participants
@Nimfadora
Contributor

Nimfadora commented Jul 15, 2017

sub-task of #4681

full report is here

in scope of this issue we should fix:
J2ME
Java 7
Java 8
Java language level issues
Java language level migration aids
JavaBeans issues
Javadoc issues
JUnit issues

Violations included:

EqualsReplaceableByObjectsCall
AbstractClassWithOnlyOneDirectInheritor
ArrayLengthInLoopCondition
InterfaceWithOnlyOneDirectInheritor
CheckForOutOfMemoryOnLargeArrayAllocation
MethodCallInLoopCondition
PrivateMemberAccessBetweenOuterAndInnerClass
Annotation
AutoBoxing
AutoUnboxing
EnumClass
ForeachStatement
IfCanBeSwitch
ClassWithoutConstructor
ClassWithoutNoArgConstructor
FieldHasSetterButNoGetter
CodeBlock2Expr
MissingPackageInfo
SimplifiableIfStatement
HtmlTagCanBeJavadocTag
@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.

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.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 15, 2017

Member

Missing 'package-info.java'

please create files.

'if' statement may be replaced with && or || expression

all violations should be fixed.

Abstract class which has only one direct inheritor

suppress both cases bu javadoc tag

Array.length in loop condition

suppress: "we see no harm for us for such usages"

Interface which has only one direct inheritor

we are a library, we do not know all third-party implementations

Large array allocation with no OutOfMemoryError check

change limit to:1024

Method call in loop condition

suppress: "this rule is too sever, it requires some options to skip well known simple methods like string.length() ....."

Synthetic accessor call

suppress: "we do not like suggested style, but we could change out mind in future"

Report only null safe 'equals'

activate option " Report only null safe 'equals' " and fix the only violation.

Annotation

suppress: "this rule is inspection is only for old java, we are ok to use annotations"

Auto-boxing
Auto-unboxing

suppress: "we are ok to use auto-(un)boxing as we use modern java"

Enumerated class

suppress: "we are ok to use enumeration as we use modern java"

Extended 'for' statement

suppress: "we are ok to use for-each as we use modern java"

'if' replaceable with 'switch'

suppress: "till we switch to jacoco we cannot use this as it conflicts with policy of 100% coverage"

Class without constructor (Errors) (486)
Class without no-arg constructor (Errors) (124)
Field has setter but no getter (Errors) (312)

suppress: "we do not like suggested style"

... can be replaced with {@code ...}

ok, please apply suggested update. But in separate commit as there are a lot of changes.

Message missing on JUnit assertion

please provide fixes for all that violations, please do this in separate PRs, as amount of changes are expected to be huge.

Statement lambda can be replaced with expression lambda

suppress: "decision to suppress was only a matter of habit to see code in more old style with extra curly braces, we might change our mind in future."

Member

romani commented Jul 15, 2017

Missing 'package-info.java'

please create files.

'if' statement may be replaced with && or || expression

all violations should be fixed.

Abstract class which has only one direct inheritor

suppress both cases bu javadoc tag

Array.length in loop condition

suppress: "we see no harm for us for such usages"

Interface which has only one direct inheritor

we are a library, we do not know all third-party implementations

Large array allocation with no OutOfMemoryError check

change limit to:1024

Method call in loop condition

suppress: "this rule is too sever, it requires some options to skip well known simple methods like string.length() ....."

Synthetic accessor call

suppress: "we do not like suggested style, but we could change out mind in future"

Report only null safe 'equals'

activate option " Report only null safe 'equals' " and fix the only violation.

Annotation

suppress: "this rule is inspection is only for old java, we are ok to use annotations"

Auto-boxing
Auto-unboxing

suppress: "we are ok to use auto-(un)boxing as we use modern java"

Enumerated class

suppress: "we are ok to use enumeration as we use modern java"

Extended 'for' statement

suppress: "we are ok to use for-each as we use modern java"

'if' replaceable with 'switch'

suppress: "till we switch to jacoco we cannot use this as it conflicts with policy of 100% coverage"

Class without constructor (Errors) (486)
Class without no-arg constructor (Errors) (124)
Field has setter but no getter (Errors) (312)

suppress: "we do not like suggested style"

... can be replaced with {@code ...}

ok, please apply suggested update. But in separate commit as there are a lot of changes.

Message missing on JUnit assertion

please provide fixes for all that violations, please do this in separate PRs, as amount of changes are expected to be huge.

Statement lambda can be replaced with expression lambda

suppress: "decision to suppress was only a matter of habit to see code in more old style with extra curly braces, we might change our mind in future."

@Nimfadora Nimfadora changed the title from idea: java lang specific issues to idea: seventh part of idea violations Jul 16, 2017

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

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

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

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Jul 17, 2017

Contributor

@romani AssertsWithoutMessages 1040 cases, really? I think we should put it away for a while and create separate issue.

Contributor

Nimfadora commented Jul 17, 2017

@romani AssertsWithoutMessages 1040 cases, really? I think we should put it away for a while and create separate issue.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 17, 2017

Member

AssertsWithoutMessages 1040 cases, really? I think we should put it away for a while and create separate issue.

you already did some fixes for PMD,
but yes this is unfortunate well spread bad practice that we need stop now.
The more we wait the more bad code be produced by us.

Member

romani commented Jul 17, 2017

AssertsWithoutMessages 1040 cases, really? I think we should put it away for a while and create separate issue.

you already did some fixes for PMD,
but yes this is unfortunate well spread bad practice that we need stop now.
The more we wait the more bad code be produced by us.

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

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue Jul 18, 2017

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

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

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 20, 2017

Member

one more part is merged, please regenerate report on TC.

Member

romani commented Jul 20, 2017

one more part is merged, please regenerate report on TC.

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Jul 20, 2017

Contributor

@romani three checks left
AssertsWithoutMessages
HtmlTagCanBeJavadocTag
EqualsReplaceableByObjectsCall
report on the 2 and 3 will be submitted today;
first was discussed.

Contributor

Nimfadora commented Jul 20, 2017

@romani three checks left
AssertsWithoutMessages
HtmlTagCanBeJavadocTag
EqualsReplaceableByObjectsCall
report on the 2 and 3 will be submitted today;
first was discussed.

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

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

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

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

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

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

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

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

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Jul 23, 2017

Contributor

@romani after PR merge task may be closed, AssertsWithoutMessages will be addressed in #4808

Contributor

Nimfadora commented Jul 23, 2017

@romani after PR merge task may be closed, AssertsWithoutMessages will be addressed in #4808

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

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 24, 2017

Member

Closed

Member

romani commented Jul 24, 2017

Closed

@romani romani closed this Jul 24, 2017

@romani romani added this to the 8.1 milestone Jul 24, 2017

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

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