Skip to content
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

[java] Replace FieldDeclarationsShouldBeAtStartOfClass with GroupInitializationLogic #3228

Open
oowekyala opened this issue Apr 19, 2021 · 0 comments
Labels
a:new-rule Proposal to add a new built-in rule
Milestone

Comments

@oowekyala
Copy link
Member

oowekyala commented Apr 19, 2021

Proposed Rule Name: GroupInitializationLogic

Proposed Category: Code Style

Description: I think the motivation for FieldDeclarationsShouldBeAtStartOfClass is to group the initialization logic for the object. That should mean,

  • that all instance fields, instance initializers and constructors are adjacent. All initializers should precede all constructors. Order of initializers and fields is however irrelevant.
  • that all static fields and static initializers are adjacent.

Whether those are at the top of the class or not is irrelevant, but we could constrain this with a property. This way we would also subsume FieldDeclarationsShouldBeAtStartOfClass.

The current problem I see with FieldDeclarationsShouldBeAtStartOfClass, is that it has a few properties that make exceptions for some things (enums, interfaces, fields initialized with an anonymous class) and not others. Their defaults are inconsistent, and also, what about classes, records, etc? Ultimately I think the rule doesn't need those properties at all. Allowing any interface or enum before fields is just contrary to the code style the rule tries to enforce. Refs #345, sourceforge/1126.

Initially I wrote an enhancement ticket with the goal to remove those properties from the rule. I now think it makes more sense to avoid this altogether and replace the rule with a better one.

Code Sample: This should include code, that should be flagged by the rule. If possible, the "correct" code
according to this new rule should also be demonstrated.

// FieldDeclarationsShouldBeAtStartOfClass would tell you this is ok
// GroupInitializationLogic would flag the interface declaration
class Foo {
   int i;
   interface Random {}

   Foo() {}
   Foo(int  i) { this.i = i; }
}
// FieldDeclarationsShouldBeAtStartOfClass would tell you this is ok, if you set up the properties correctly
// GroupInitializationLogic would tell you this is ok because the initialization logic is not broken up
class Foo {
   interface Random {}

   int i;
   Foo() {}
   Foo(int  i) { this.i = i; }
}

Possible Properties:

  • requireAtStartOfClass: constrains the initialization logic to be at the very start of the class. This would make the rule a more principled version of FieldDeclarationsShouldBeAtStartOfClass.
@oowekyala oowekyala added the a:new-rule Proposal to add a new built-in rule label Apr 19, 2021
@oowekyala oowekyala mentioned this issue Apr 7, 2022
55 tasks
@adangel adangel added this to the 7.0.0 milestone Jan 9, 2023
@adangel adangel modified the milestones: 7.0.0, 7.x Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:new-rule Proposal to add a new built-in rule
Projects
None yet
Development

No branches or pull requests

2 participants