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: third part of violations #4713

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:
Duplicate String Literal 8645
Hard coded strings 11888
String concatenation 9477
Single class import 4567
Static import 705

report on all other violations

violations included:

AbstractClassExtendsConcreteClass
BooleanMethodIsAlwaysInverted
BooleanVariableAlwaysNegated
BreakStatement
ContinueStatement
CyclicClassDependency
CyclicPackageDependency
DuplicateStringLiteralInspection
EmptyMethod
ErrorRethrown
HardCodedStringLiteral
IfMayBeConditional
LawOfDemeter
MagicCharacter
NonThreadSafeLazyInitialization
PublicInnerClass
SameParameterValue
SameReturnValue
SingleClassImport
StaticImport
StringConcatenation
NestedTryStatement
ProtectedField
ProtectedInnerClass
AssignmentToForLoopParameter
ForLoopWithMissingComponent
CheckedExceptionClass
ExtendsConcreteCollection
StaticVariableInitialization
StaticVariableUninitializedUse
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 13, 2017

Member

Duplicate String Literal

suppress: it will result in too much dependencies between classes, some case are only have the same value by chance, too much wont-fixes.

Hard coded strings
String concatenation

suppress: we like it, it is not performance issue for us.

Single class import

suppress: we like single import for single class

Static import

exclude Test scope, it should result in no violations.

Member

romani commented Jul 13, 2017

Duplicate String Literal

suppress: it will result in too much dependencies between classes, some case are only have the same value by chance, too much wont-fixes.

Hard coded strings
String concatenation

suppress: we like it, it is not performance issue for us.

Single class import

suppress: we like single import for single class

Static import

exclude Test scope, it should result in no violations.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 13, 2017

Member

AssignmentToForLoopParameter

need to refactored to "while"

BreakStatement

suppress: "we like breaks to be in code"

ContinueStatement

suppress: "we like continue statements to be in code"

ForLoopWithMissingComponent

activate the only option in it, all violations that are left should be addressed.

IfMayBeConditional

suppress: "we do not conditional expressions, we like more verbose code"

BooleanMethodIsAlwaysInverted
BooleanVariableAlwaysNegated

suppress: "even it is always negated, it is better than possible negation/inversion of negated term"

LawOfDemeter

suppress: "it is impossible to follow this Law for us as most violation are caused by fact that we do logic not only on token that come to visitToken, but by extra traversing over subtree"

SameParameterValue

suppress: "to much false positives as it does not consider fact that public method is called from outside of class and values come from config files and it highlight most of static methods that are candidates for utils package but we not decided yet......."

EmptyMethod

activate option to count comment as content and should result in no violations

SameReturnValue

suppress: "it is ok as some methods are in role of configuration 'isEditable{ return false;}' "

CyclicClassDependency
CyclicPackageDependency

suppress: "we can not afford this now, api cleanup is in progress and it hard and takes time for deprecation process......"

ProtectedField

resolve by makin git private and share it by public method getBriefUtLogger

ProtectedInnerClass

suppress these violations by javadoc annotation , such classes are deprecated together with their top class

PublicInnerClass

suppress: "we like to use them as we not always ready to make class top level, for Checks it is better to keep all in one class."

CheckedExceptionClass

suppress the only violation by javadoc annotation, it is the api class, it hard to change it. But no other class should appear

ErrorRethrown

Suppress: "there is false-positive, and rest cases are result of our loading modules by reflection that throws Error. Error is not always jvm problem, bunch of libraries throw Error instead of Exception"

NestedTryStatement

We need to fix this cases.

UnusedCatchParameter

with its the only property enabled, should produce no result.

AbstractClassExtendsConcreteClass

suppress till #4716

ExtendsConcreteCollection

suppress the only case by javadoc annotation.

MagicCharacter

suppress: we do a lot of String processing ; we have lexer and parser, we do not have java class that contains all lexer tokens with their text values(RPAREN vs ')'),

InstanceVariableInitialization

suppress: "we use methods like notifyBegin to (re)initialize, so c-tor or field declaration initialization is not good place for us."

StaticVariableInitialization
StaticVariableUninitializedUse

suppress in test scope, there will be violation only FileContentsHolder that is deprecated and will be removed soon, so put on it javadoc annotation for suppression of violation

Member

romani commented Jul 13, 2017

AssignmentToForLoopParameter

need to refactored to "while"

BreakStatement

suppress: "we like breaks to be in code"

ContinueStatement

suppress: "we like continue statements to be in code"

ForLoopWithMissingComponent

activate the only option in it, all violations that are left should be addressed.

IfMayBeConditional

suppress: "we do not conditional expressions, we like more verbose code"

BooleanMethodIsAlwaysInverted
BooleanVariableAlwaysNegated

suppress: "even it is always negated, it is better than possible negation/inversion of negated term"

LawOfDemeter

suppress: "it is impossible to follow this Law for us as most violation are caused by fact that we do logic not only on token that come to visitToken, but by extra traversing over subtree"

SameParameterValue

suppress: "to much false positives as it does not consider fact that public method is called from outside of class and values come from config files and it highlight most of static methods that are candidates for utils package but we not decided yet......."

EmptyMethod

activate option to count comment as content and should result in no violations

SameReturnValue

suppress: "it is ok as some methods are in role of configuration 'isEditable{ return false;}' "

CyclicClassDependency
CyclicPackageDependency

suppress: "we can not afford this now, api cleanup is in progress and it hard and takes time for deprecation process......"

ProtectedField

resolve by makin git private and share it by public method getBriefUtLogger

ProtectedInnerClass

suppress these violations by javadoc annotation , such classes are deprecated together with their top class

PublicInnerClass

suppress: "we like to use them as we not always ready to make class top level, for Checks it is better to keep all in one class."

CheckedExceptionClass

suppress the only violation by javadoc annotation, it is the api class, it hard to change it. But no other class should appear

ErrorRethrown

Suppress: "there is false-positive, and rest cases are result of our loading modules by reflection that throws Error. Error is not always jvm problem, bunch of libraries throw Error instead of Exception"

NestedTryStatement

We need to fix this cases.

UnusedCatchParameter

with its the only property enabled, should produce no result.

AbstractClassExtendsConcreteClass

suppress till #4716

ExtendsConcreteCollection

suppress the only case by javadoc annotation.

MagicCharacter

suppress: we do a lot of String processing ; we have lexer and parser, we do not have java class that contains all lexer tokens with their text values(RPAREN vs ')'),

InstanceVariableInitialization

suppress: "we use methods like notifyBegin to (re)initialize, so c-tor or field declaration initialization is not good place for us."

StaticVariableInitialization
StaticVariableUninitializedUse

suppress in test scope, there will be violation only FileContentsHolder that is deprecated and will be removed soon, so put on it javadoc annotation for suppression of violation

@romani romani added this to In Progress in Practice What You Preach Jul 15, 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 added a commit to Nimfadora/checkstyle that referenced this issue Jul 19, 2017

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Jul 19, 2017

Contributor

@romani after merge of current pull request task may be closed, all violations are fixed

Contributor

Nimfadora commented Jul 19, 2017

@romani after merge of current pull request task may be closed, all violations are fixed

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

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

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

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

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

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Jul 21, 2017

Contributor

@romani task may be closed, all violations are fixed

Contributor

Nimfadora commented Jul 21, 2017

@romani task may be closed, all violations are fixed

@romani romani closed this Jul 21, 2017

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

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

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