Expand checkstyle to all modules (root)

[lprimak]

Currently, only Jakarta EE module has checkstyle enabled.
Need to enable checkstyle on root module, so it encompasses all modules.
Of course, this requires that all modules comply with the checkstyle rules.
This needs to be done as well.

[fpapon]

Just a warning, if we want to enable checkstyle, we could have a lot of code source update to do.

[EdgarRamirezFuentes]

Hi, guys!
I'm new in this world of Java, but I would like to keep contributing to this project.
If you guide through this issue, I am willing to learn and do the research to help in this project. :D

[bmarwell]

Hi @EdgarRamirezFuentes

Probably an inherit parameter is enough between the version and the config right here: https://github.com/apache/shiro/blob/main/pom.xml#L543-L546

But haven't looked into it, as I am in a rush right now.

If it works, you should get a message that checkstyle was executed for every module when running mvn verify on Shiro. It will probably break something too.

If you get to that point, create the PR and point out what did break and why. If there's just small fixes needed (eg whitespace changes), just do a second commit with the fixes. In any other case, we might want to exclude some modules from checking or just do a big "fix up all" commit and add it to a special ignore file. Yes, the commit to that file needs to be a separate one. :)

HTH to get you started!

[lprimak]

Probably an inherit parameter

Please disregard the above (sorry, that's incorrect @bmarwell :)

Instead, move the section mentioning the checkstyle plugin from

shiro/integration-tests/jakarta-ee/pom.xml

Line 472 in d627bf6

<plugin>

to the <plugin> section of the root pom.
This will activate checkstyle at the root level, and as you might have guessed, break a lot of things because they don't conform to checkstyle rules :)
Those would need to be fixed as they come up.

Once again, thank you!

[bmarwell]

Yes, I caught the plugin management section. Oops.

[EdgarRamirezFuentes]

@lprimak @bmarwell I hope you're having a nice day!
I moved the checkstyle to the root pom, and as it was expected... it broke a lot of things.
In the PR I added photos to show you the result of running mvn verify.
I'll be working on fixing some of the code that does not fulfill the checkstyle rules, and i'll commit those changes.

[EdgarRamirezFuentes]

@lprimak @bmarwell Hi! It's me again.
I've got a question...
There are some checkstyle rules errors related to magic number, such as:
\shiro\lang\src\main\java\org\apache\shiro\lang\codec\H64.java:53:29: '0xff' is a magic number. [MagicNumber]

private static short toShort(byte b) {
      return (short) (b & 0xff);
}

or

\shiro\lang\src\main\java\org\apache\shiro\lang\codec\H64.java:57:40: '4' is a magic number. [MagicNumber]

if (numBytes < 1 || numBytes > 4) {
    throw new IllegalArgumentException("numBytes must be between 1 and 4.");
}

Should I add variable to fix this magic numbers?

At the moment I just fixed easy checkstyle rules errors such as:

• \shiro\lang\src\main\java\org\apache\shiro\lang\codec\CodecSupport.java:100:91: '+' should be on a new line. [OperatorWrap]
• \shiro\lang\src\main\java\org\apache\shiro\lang\codec\CodecSupport.java:184:82: '||' should be on a new line. [OperatorWrap]
• \shiro\lang\src\main\java\org\apache\shiro\lang\codec\CodecSupport.java:307:101: '+' should be on a new line. [OperatorWrap]
• \shiro\lang\src\main\java\org\apache\shiro\lang\codec\CodecSupport.java:308:105: '+' should be on a new line. [OperatorWrap]
• \shiro\lang\src\main\java\org\apache\shiro\lang\codec\CodecSupport.java:309:77: '+' should be on a new line. [OperatorWrap]
• \shiro\lang\src\main\java\org\apache\shiro\lang\codec\CodecSupport.java:310:104: '+' should be on a new line. [OperatorWrap]
• \shiro\lang\src\main\java\org\apache\shiro\lang\codec\CodecSupport.java:312:108: '+' should be on a new line. [OperatorWrap]

I'll keep working on fixing checkstyle rules such as the listed before.

Have a nice day!

[lprimak]

I left some comments on the PR, please continue there.

[damonxue]

pls assign 2 me.

[lprimak]

@EdgarRamirezFuentes What do you think about below?

pls assign 2 me.

[EdgarRamirezFuentes]

@lprimak Sure! I have no problem about it.
At this moment it's kinda hard to me to contribute to the project. So, It would be nice if @damonxue helps us to fix this issue.

[lprimak]

Done. @EdgarRamirezFuentes Can you give @damonxue permissions to your PR?
Alternatively, Damon can create your own.

[damonxue]

Done. @EdgarRamirezFuentes Can you give @damonxue permissions to your PR?

Alternatively, Damon can create your own.

Pls wait for me, I will done a task in Apache Shenyu in a week,and then do this.

[damonxue]

@lprimak Pls review.