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: fifth part of idea violations #4726

Closed
Nimfadora opened this Issue Jul 15, 2017 · 7 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:
Performance issues
Portability issues
Probable bugs
Properties files

Violations included:

fixed:
EqualsUsesNonFinalVariable
HashCodeUsesNonFinalVariable
AlphaUnsortedPropertiesFile
InconsistentResourceBundle
MethodMayBeStatic
CallToSimpleGetterInClass
MismatchedCollectionQueryUpdate
NullableProblems
CollectionsMustHaveInitialCapacity
ConstantConditions
DuplicatePropertyInspection
HardcodedFileSeparators
HardcodedLineSeparators
LengthOneStringsInConcatenation
MagicConstant
ObjectAllocationInLoop
ObjectEquality
ReturnNull
SuspiciousArrayCast

StringBufferMustHaveInitialCapacity will be addressed #4812

@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 16, 2017

Member

@Nimfadora , I already did a review of such violations .... why you mix inspections between PR ?
see reply at #4725

Member

romani commented Jul 16, 2017

@Nimfadora , I already did a review of such violations .... why you mix inspections between PR ?
see reply at #4725

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

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Jul 17, 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

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 20, 2017

Member

I still do not see pull request and report on this

Member

romani commented Jul 20, 2017

I still do not see pull request and report on this

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Jul 20, 2017

Contributor

@romani it is stated in the task text as todo, all of them have your comments, so soon it will be fixed

Contributor

Nimfadora commented Jul 20, 2017

@romani it is stated in the task text as todo, all of them have your comments, so soon it will be fixed

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Jul 20, 2017

Contributor

EqualsUsesNonFinalVariable
HashCodeUsesNonFinalVariable

Here you said that for SuppressElement issue can be resolved by making variables final and creating c-tors. But some of variables used in equals and hashCode have setters (linesCsv, columnsCsv, moduleId), do we have to remove them and rewrite the logic in couple of classes for that purpose?

Contributor

Nimfadora commented Jul 20, 2017

EqualsUsesNonFinalVariable
HashCodeUsesNonFinalVariable

Here you said that for SuppressElement issue can be resolved by making variables final and creating c-tors. But some of variables used in equals and hashCode have setters (linesCsv, columnsCsv, moduleId), do we have to remove them and rewrite the logic in couple of classes for that purpose?

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

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 21, 2017

Member

do we have to remove them and rewrite the logic in couple of classes for that purpose?

If update is simple - yes.
As far as I remember all setters are used in one place of cod, so refactoring should be easy to keep all parts of object in local variables create class only when all information is available.

Member

romani commented Jul 21, 2017

do we have to remove them and rewrite the logic in couple of classes for that purpose?

If update is simple - yes.
As far as I remember all setters are used in one place of cod, so refactoring should be easy to keep all parts of object in local variables create class only when all information is available.

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

Issue #4726: fix CallToSimpleGetterInClass, MismatchedCollectionQuery…
…Update, NullableProblems idea violations

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 23, 2017

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

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

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

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

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

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

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

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

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

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

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

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

@romani romani added this to the 8.2 milestone Aug 8, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Aug 8, 2017

Member

fix is merged

Member

romani commented Aug 8, 2017

fix is merged

@romani romani closed this Aug 8, 2017

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

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