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

Updates to support groovy 4 #439

Merged
merged 2 commits into from
Dec 15, 2024
Merged

Updates to support groovy 4 #439

merged 2 commits into from
Dec 15, 2024

Conversation

AngMits
Copy link
Contributor

@AngMits AngMits commented Dec 7, 2024

Fixes

  • Parsing Groovy 4 switch statements.
  • I have been facing issues with parsing switch statements after upgrading work-related projects to Groovy 4. The current state of the linter will throw a parser error if it encounters such a statement.
  • Instead of adding exclusions for each case, adding support for the language would be a slightly better alternative.

Proposed Changes

  • Update jars to their latest versions (commons)
  • Update jars to support Groovy 4

Additional info ℹ️

  • Although local tests are passing, and a trial run using the actions showed that nothing is broken, please request any changes that might be required or any other course of action.
  • The added import in the CodeNarcServer was required, otherwise Groovy would throw an error.
  • The update of the workflow file was done to check the workflows, otherwise, an error version would be thrown.

Bump jars to their latest versions
Update the CodeNarcServer
Update linter groovy version
@nvuillam
Copy link
Owner

nvuillam commented Dec 7, 2024

Amazing, i'll test tht asap :)

cc @stevenh

@stevenh
Copy link
Collaborator

stevenh commented Dec 10, 2024

Just a quick note to say I've seen this and its on my list to look at.

@AngMits
Copy link
Contributor Author

AngMits commented Dec 11, 2024

I believe a minimal example could be useful here for testing. Instead of updating the description, I'm adding this working code snippet as a comment. More complex scenarios benefit from this syntax, but this should be sufficient, as it reflects a somewhat reasonable use of a Groovy class that features return statements.

class TestClass {

    enum TestStrings{
        ONE('One has been used'),
        TWO('Two has been used'),
        DEFAULT('Default string')
  
        String content
  
        TestStrings(String content) {
            this.content = content
        }

        String get() {
            content
        }
    }

    static String compareStrings(String testString) {
 
        String innerString = TestStrings.DEFAULT.get()

        switch (testString) {
            case 'one' -> {
                innerString = TestStrings.ONE.get()
                println(innerString)
            }
            case 'two' -> {
                innerString = TestStrings.TWO.get()
                println(innerString)
            }
           default -> println(innerString)
        }
        innerString
    }
}

assert TestClass.compareStrings() == TestClass.TestStrings.DEFAULT.get()
assert TestClass.compareStrings('one') == TestClass.TestStrings.ONE.get()
assert TestClass.compareStrings('two') == TestClass.TestStrings.TWO.get()

Copy link
Collaborator

@stevenh stevenh left a comment

Choose a reason for hiding this comment

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

Looks like just an editor format change and then should be good to go.

groovy/src/main/com/nvuillam/CodeNarcServer.groovy Outdated Show resolved Hide resolved
@AngMits AngMits requested a review from stevenh December 12, 2024 17:04
Copy link
Collaborator

@stevenh stevenh left a comment

Choose a reason for hiding this comment

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

LGTM

@nvuillam nvuillam merged commit 4d15bc9 into nvuillam:main Dec 15, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants