Skip to content

Remove calls to no-arg superclass constructor #3132

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

Closed
strkkk opened this issue Dec 25, 2019 · 10 comments · Fixed by #3134
Closed

Remove calls to no-arg superclass constructor #3132

strkkk opened this issue Dec 25, 2019 · 10 comments · Fixed by #3134
Assignees
Milestone

Comments

@strkkk
Copy link

strkkk commented Dec 25, 2019

Hi,

In checkstyle project we adding a new check to detect calls to no-arg superclass constructor super();
checkstyle/checkstyle#7387
Our analysis shows that there are many violations of this check in your source code.
Analysis report:
https://strkkk.github.io/checkstyle/7363_3/spring-integration/index.html#A1

Can you please share your thoughts - it this calls should be removed or they can be kept for some reason?

Any feedback are appreciated.

@garyrussell
Copy link
Contributor

garyrussell commented Dec 25, 2019

There are other "rules" about empty blocks; our Sonar ruleset barks about them so an empty constructor is a violation of that rule.

There is also the EmptyBlock checkstyle rule.

We use super() only when a no-arg ctor is needed without extra code.

The rule you cite seems bogus to me - if you look at the byte code generated, the compiler adds the same super() implicitly.

@garyrussell garyrussell added the status: waiting-for-reporter Needs a feedback from the reporter label Dec 25, 2019
@garyrussell
Copy link
Contributor

If you can make the rule only trigger if there is extra code in the constructor, it might then be useful.

@strkkk
Copy link
Author

strkkk commented Dec 25, 2019

Thank you for reply.

The rule you cite seems bogus to me - if you look at the byte code generated, the compiler adds the same super() implicitly.

Yeah, this is the reason why explicit call is redundant and goal of the check is to detect it.

@artembilan
Copy link
Member

Well, I think we can change that super() call into a comment to satisfy all the rules around.
Also I think no one of them are aware about private empty ctors.

@garyrussell
Copy link
Contributor

More discussion here about possibly changing EmptyBlock to allow empty no-arg CTORs.

@artembilan
Copy link
Member

OK. So, placing "on-hold" until a resolution for linked issue in Checkstyle.

@strkkk
Copy link
Author

strkkk commented Dec 27, 2019

@garyrussell

There is also the EmptyBlock checkstyle rule.

I checked this checkstyle rule and there is nothing about checking ctors. Can you please share checkstyle config that reports violation on empty ctor?

@garyrussell
Copy link
Contributor

Interesting - I see checkstyle doesn't complain about empty CTORs and I just ran a test with our current Sonar rules and that passed too

https://sonar.spring.io/code?id=org.springframework.kafka%3Aspring-kafka-dist%3Asonar&selected=org.springframework.kafka%3Aspring-kafka-dist%3Asonar%3Aspring-kafka%2Fsrc%2Fmain%2Fjava%2Forg%2Fspringframework%2Fkafka%2Flistener%2FFoo.java

So this must have been some old rule on an older Sonar installation that is no longer triggered.

I therefore withdraw my objection.

@artembilan
Copy link
Member

So, do we remove super(); from our classes to satisfy requirements here?

@garyrussell
Copy link
Contributor

I would say so, yes (over time).

@artembilan artembilan added type: enhancement and removed status: waiting-for-reporter Needs a feedback from the reporter labels Dec 27, 2019
@artembilan artembilan self-assigned this Dec 27, 2019
@artembilan artembilan added this to the 5.3.0.M1 milestone Dec 27, 2019
artembilan added a commit to artembilan/spring-integration that referenced this issue Dec 27, 2019
Fixes spring-projects#3132

It turns out that Checkstyle EmptyBlock doesn't complain about
empty default ctor.
Plus a new check for `super();` call treats it as a violation

* Remove `super();` from all the no-arg ctors
* Code style clean up in the affected classes according
IDEA suggestions
artembilan added a commit to artembilan/spring-integration that referenced this issue Dec 27, 2019
Fixes spring-projects#3132

It turns out that Checkstyle EmptyBlock doesn't complain about
empty default ctor.
Plus a new check for `super();` call treats it as a violation

* Remove `super();` from all the no-arg ctors
* Code style clean up in the affected classes according
IDEA suggestions

* Fix new Sonar smells
garyrussell pushed a commit that referenced this issue Dec 27, 2019
Fixes #3132

It turns out that Checkstyle EmptyBlock doesn't complain about
empty default ctor.
Plus a new check for `super();` call treats it as a violation

* Remove `super();` from all the no-arg ctors
* Code style clean up in the affected classes according
IDEA suggestions

* Fix new Sonar smells
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants