Skip to content

Issue #757: Bumped Codenarc to version v2.2.0#836

Merged
nrmancuso merged 1 commit into
checkstyle:masterfrom
MANISH-K-07:codenarc
Feb 27, 2024
Merged

Issue #757: Bumped Codenarc to version v2.2.0#836
nrmancuso merged 1 commit into
checkstyle:masterfrom
MANISH-K-07:codenarc

Conversation

@MANISH-K-07

@MANISH-K-07 MANISH-K-07 commented Feb 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Aims to close Issue #757

The version of codenarc being used was outdated through v1.5
As part of the commit, I have bumped codenarc version to highest compatible v2.2.0

var_names in file diff.groovy were found violating VariableName rule. These have been modified to adhere to checkstyle rules.

Prior changes :

    final REPO_NAME_PARAM_NO = 0
    final REPO_TYPE_PARAM_NO = 1
    final REPO_URL_PARAM_NO = 2
    final REPO_COMMIT_ID_PARAM_NO = 3
    final REPO_EXCLUDES_PARAM_NO = 4
    final FULL_PARAM_LIST_SIZE = 5

After changes :

    final repoNameParamNo = 0
    final repoTypeParamNo = 1
    final repoURLParamNo = 2
    final repoCommitIDParamNo = 3
    final repoExcludesParamNo = 4
    final fullParamListSize = 5

@MANISH-K-07 MANISH-K-07 marked this pull request as ready for review February 22, 2024 11:34
@MANISH-K-07

MANISH-K-07 commented Feb 22, 2024

Copy link
Copy Markdown
Contributor Author

The error is :

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.codehaus.groovy.reflection.CachedClass (file:/usr/share/groovy/lib/groovy-2.4.21.jar) to method java.lang.Object.finalize()
WARNING: Please consider reporting this to the maintainers of org.codehaus.groovy.reflection.CachedClass
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
java.lang.AbstractMethodError: Receiver class org.codenarc.CodeNarc does not define or inherit an implementation of the resolved method 'abstract void setProperty(java.lang.String, java.lang.Object)' of interface groovy.lang.GroovyObject.

@nrmancuso ,
I am not sure if this is true, but I don't think a workaround to the above error exists with the current groovy version that checkstyle uses ( 2.4.x stream )

We could suppress the java Illegal reflective access warnings to satisfy the CI
What do you suggest ?

@nrmancuso

nrmancuso commented Feb 22, 2024

Copy link
Copy Markdown
Contributor

Let’s find the compatibility matrix for groovy version/codenarc and answer two questions:

  1. What is the highest codenarc version we can currently use?
  2. What version of groovy do we need to be on to use the latest codenarc?

@MANISH-K-07

MANISH-K-07 commented Feb 22, 2024

Copy link
Copy Markdown
Contributor Author

What is the highest codenarc version we can currently use?

The latest codenarc version released is v3.4.0 which I have used at present.

What version of groovy do we need to be on to use the latest codenarc?

I believe we require Groovy version 3.0 or later to be compatible with codenarc bump

PS : the latest version of groovy is 4.0

@nrmancuso

nrmancuso commented Feb 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

What is the highest codenarc version we can currently use?

The latest codenarc version released is v3.4.0 which I have used at present.

What version of groovy do we need to be on to use the latest codenarc?

I believe we require Groovy version 3.0 or later to be compatible with codenarc bump

PS : the latest version of groovy is 4.0

This response does not answer either of my questions. Please find some compatibility matrix or other resource (release notes) that specifies this information.

@MANISH-K-07

MANISH-K-07 commented Feb 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

This response does not answer either of my questions. Please find some compatibility matrix or other resource (release notes) that specifies this information.

I couldn't find the compatibility matrix for either. The following is the info from release notes and docs :

v3.4.0 Latest (codenarc release notes)

Requirements
CodeNarc requires:
Java 1.8 or later
[Groovy](http://groovy-lang.org/) version 3.0 or later; Groovy version 4.0 or later for the 3.x.x-groovy-4.x versions of CodeNarc
[SLF4J](https://www.slf4j.org/) API and binding jar(s)
[GMetrics](https://dx42.github.io/gmetrics/) or GMetrics-Groovy4 2.1.0 or later, if using the Size/Complexity rules

The requirement of groovy 3 is evident in discussion at CodeNarc/CodeNarc#630 also.

Please let me know if you require any other info that specifies with further clarity.

@MANISH-K-07

MANISH-K-07 commented Feb 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

<-- Not related to this Issue -->

While browsing for the said info, I noticed that the checkstyle compatibility matrix page hasn't been updated since 2020.
Latest major update: 2020-02-29

The highest version specified in the html is outdated through release 8.29

@nrmancuso

nrmancuso commented Feb 22, 2024

Copy link
Copy Markdown
Contributor

Still looking for an answer to:

What is the highest codenarc version we can currently use?

@MANISH-K-07 MANISH-K-07 force-pushed the codenarc branch 3 times, most recently from 2416c18 to a7c0e7c Compare February 23, 2024 07:57
@MANISH-K-07

MANISH-K-07 commented Feb 23, 2024

Copy link
Copy Markdown
Contributor Author

Still looking for an answer to:

What is the highest codenarc version we can currently use?

So, the versions from v3.0.0 and higher reproduce the error mentioned in #836 (comment)

The highest possible version that doesn't give this error is v2.2.0
There are however other violations that arise :

Summary: TotalFiles=1 FilesWithViolations=1 P1=0 P2=6 P3=0
File: diff.groovy
    Violation: Rule=VariableName P=2 Line=257 Msg=[Variable named REPO_NAME_PARAM_NO in class None does not match the pattern [a-z][a-zA-Z0-9]*] Src=[final REPO_NAME_PARAM_NO = 0]
    Violation: Rule=VariableName P=2 Line=258 Msg=[Variable named REPO_TYPE_PARAM_NO in class None does not match the pattern [a-z][a-zA-Z0-9]*] Src=[final REPO_TYPE_PARAM_NO = 1]
    Violation: Rule=VariableName P=2 Line=259 Msg=[Variable named REPO_URL_PARAM_NO in class None does not match the pattern [a-z][a-zA-Z0-9]*] Src=[final REPO_URL_PARAM_NO = 2]
    Violation: Rule=VariableName P=2 Line=260 Msg=[Variable named REPO_COMMIT_ID_PARAM_NO in class None does not match the pattern [a-z][a-zA-Z0-9]*] Src=[final REPO_COMMIT_ID_PARAM_NO = 3]
    Violation: Rule=VariableName P=2 Line=261 Msg=[Variable named REPO_EXCLUDES_PARAM_NO in class None does not match the pattern [a-z][a-zA-Z0-9]*] Src=[final REPO_EXCLUDES_PARAM_NO = 4]
    Violation: Rule=VariableName P=2 Line=262 Msg=[Variable named FULL_PARAM_LIST_SIZE in class None does not match the pattern [a-z][a-zA-Z0-9]*] Src=[final FULL_PARAM_LIST_SIZE = 5]

[CodeNarc (https://www.codenarc.org) v2.2.0]
CodeNarc completed: (p1=0; p2=6; p3=0) 4687ms

Could these be suppressed ?

@MANISH-K-07

MANISH-K-07 commented Feb 25, 2024

Copy link
Copy Markdown
Contributor Author

@nrmancuso , please suggest

@nrmancuso

nrmancuso commented Feb 25, 2024

Copy link
Copy Markdown
Contributor

@nrmancuso , please suggest

We can just fix them, since we are just renaming variables

@MANISH-K-07 MANISH-K-07 changed the title Issue #757: Bumped codenarc to latest version v3.4.0 Issue #757: Bumped Codenarc to version v2.2.0 Feb 25, 2024
@MANISH-K-07

MANISH-K-07 commented Feb 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

We can just fix them, since we are just renaming variables

@nrmancuso ,
I have modified the var_names to follow checkstyle requirements. The CI seem to be satisfied !!
Version of codenarc v2.2.0 is currently used for update and ci/circleci: codenarc validation for groovy files is happy.
Please review....

Also, Could I help in any way regarding #836 (comment) ??

romani
romani approved these changes Feb 25, 2024
View reviewed changes

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok to merge, thanks a lot for your help

@nrmancuso nrmancuso merged commit cf18516 into checkstyle:master Feb 27, 2024
@nrmancuso nrmancuso mentioned this pull request Feb 27, 2024
Closed
@MANISH-K-07 MANISH-K-07 deleted the codenarc branch February 27, 2024 03:50
@MANISH-K-07 MANISH-K-07 mentioned this pull request Feb 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@romani romani romani approved these changes

Assignees

@nrmancuso nrmancuso

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MANISH-K-07 @nrmancuso @romani