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: sixth part of idea violations #4724

Closed
Nimfadora opened this Issue Jul 15, 2017 · 6 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:
General
Resource management issues
Security issues
Serialization issues
Spelling
TestNG
Threading issues
toString() issues
Visibility issues

Violations fixed:

AutoCloseableResource
ClassHasNoToStringMethod
CloneableClassInSecureContext
DeserializableClassInSecureContext
DesignForExtension
JUnitTestNG
LongLine
NonStaticInnerClassInSecureContext
ParameterHidingMemberVariable
SerializableClassInSecureContext
SpellCheckingInspection
FieldNotUsedInToString
ClassLoaderInstantiation
IOResource
NonSerializableFieldInSerializableClass
NonSynchronizedMethodOverridesSynchronizedMethod
SerializableHasSerializationMethods
@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 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 16, 2017

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

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 16, 2017

Member

Line is longer than allowed by code style

suppress: "no ability to skip import/package lines, wrapping them is not what we want"

AutoCloseable used without 'try'-with-resources

suppress: "we can not use try-with-resource till we switch to jacoco for code coverage"

I/O resource opened but not safely closed

we need to fix this cases.

ClassLoader instantiation

suppress the only violation by javadoc tag.
we need to work with AntClassLoader, there is no way to avoid this.

Cloneable class in secure context
Deserializable class in secure context
Non-'static' inner class in secure context
Serializable class in secure context

suppress: "this inspection is not for us"

Design for extension

suppress: "we have the same Check , and it is disabled too, as amount of old code and old design is too big to do any breaking compatibility fixes in favor to avoid violations from this inspection"

Non-serializable field in a Serializable class

suppress by javadoc tag, it will be throughly investigated at #4739 .

Serializable class without 'readObject()' and 'writeObject()'

activate all checkbox option for this inspection
set "Ignore subclasses of" for all gui classes that we have violation on.
api classes please suppress by javadoc tag , them will be addressed at #4741 .

Typo

suppress: "even we limit validation comments only, there are still a lot of violations on javadoc parameters naming , it also violates names of inspections in noinspection tags, ...."

Convert JUnit Tests to TestNG

suppress: "we need to stay on junit"

Unsynchronized method overrides synchronized method

need to be fixed.

Class does not override 'toString()' method

suppress: "we do not need that"

Field not used in 'toString()' method

valid, should be fixed

Parameter hides field

activate all options except for "Ignore for abstract method", that will result in no violations.

Member

romani commented Jul 16, 2017

Line is longer than allowed by code style

suppress: "no ability to skip import/package lines, wrapping them is not what we want"

AutoCloseable used without 'try'-with-resources

suppress: "we can not use try-with-resource till we switch to jacoco for code coverage"

I/O resource opened but not safely closed

we need to fix this cases.

ClassLoader instantiation

suppress the only violation by javadoc tag.
we need to work with AntClassLoader, there is no way to avoid this.

Cloneable class in secure context
Deserializable class in secure context
Non-'static' inner class in secure context
Serializable class in secure context

suppress: "this inspection is not for us"

Design for extension

suppress: "we have the same Check , and it is disabled too, as amount of old code and old design is too big to do any breaking compatibility fixes in favor to avoid violations from this inspection"

Non-serializable field in a Serializable class

suppress by javadoc tag, it will be throughly investigated at #4739 .

Serializable class without 'readObject()' and 'writeObject()'

activate all checkbox option for this inspection
set "Ignore subclasses of" for all gui classes that we have violation on.
api classes please suppress by javadoc tag , them will be addressed at #4741 .

Typo

suppress: "even we limit validation comments only, there are still a lot of violations on javadoc parameters naming , it also violates names of inspections in noinspection tags, ...."

Convert JUnit Tests to TestNG

suppress: "we need to stay on junit"

Unsynchronized method overrides synchronized method

need to be fixed.

Class does not override 'toString()' method

suppress: "we do not need that"

Field not used in 'toString()' method

valid, should be fixed

Parameter hides field

activate all options except for "Ignore for abstract method", that will result in no violations.

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

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

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 20, 2017

Member

I still do not see WIP pull request and report on this.
What is left unfixed ?

Member

romani commented Jul 20, 2017

I still do not see WIP pull request and report on this.
What is left unfixed ?

@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. But you have already comment it, so soon it will be fixed

Contributor

Nimfadora commented Jul 20, 2017

@romani it is stated in the task text as todo. But you have already comment it, so soon it will be fixed

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

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

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

Contributor

Nimfadora commented Jul 23, 2017

@romani after PR merge task may be closed

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

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

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 27, 2017

Member

fixes are merged

Member

romani commented Jul 27, 2017

fixes are merged

@romani romani closed this Jul 27, 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