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

Add CodeNarc linting checks #237

Open
unitysipu opened this issue Jun 26, 2020 · 5 comments
Open

Add CodeNarc linting checks #237

unitysipu opened this issue Jun 26, 2020 · 5 comments

Comments

@unitysipu
Copy link

There's a variety of potential bugs, code smells and style issues in the library and accompanying code examples that are trivial to spot and fix using a linter during development. Using the examples as they are in code that enforces linting require them to be fixed / refactored.

It helps tremendously to set a clean baseline for any future contributions and make the linting tool a prerequisite for merging, either by using a commit hook or making it part of tests to ensure good code hygiene / eliminating obvious mistakes or discrepancies in the library.

groovy-lint can be installed via npm i -g npm-groovy-lint

Documentation

Some of the rules may not apply and can be ignored by crafting a .groovylintrc.json and committing it into the repository root. It can be easily autogenerated using the vscode-groovy-lint vscode plugin.

npm-groovy-lint and an associated vscode plugin vscode-groovy-lint also have features to auto-fix trivial issues.

@nre-ableton
Copy link
Contributor

I'm a fan of CodeNarc myself and use it heavily for other Groovy projects at my work. I'm unfamiliar with this particular wrapper, but as JenkinsPipelineUnit already has Travis CI in place, feel free to submit a PR to add linting support for it. Short of that, I think that the unit tests provide enough of a safety net for us to make releases of this library.

@unitysipu
Copy link
Author

Fixing all the nags / tuning the rules for this repository is way too much commitment from me to do, there's so many when run with recommended rules. It's probably best for somebody who's familiar with the whole library to check them out and do all the appropriate fixes / ignores. While you may have good unit tests it's still valuable to have a clean linting result as it's easier to find genuine bugs from all the noise. I'm just reporting this as it's something I see as a user and a developer leveraging this library.

@nre-ableton
Copy link
Contributor

I agree that fixing all of the violations is a lot to ask, but as you said:

Some of the rules may not apply and can be ignored by crafting a .groovylintrc.json and committing it into the repository root.

The initial PR could just add support for checking with CodeNarc and suppress all warnings. Over time, people could fix the violations and remove the suppressions. Likewise, I understand if you don't have the time to contribute such a PR, in which case we appreciate the feature request nonetheless.

@nre-ableton
Copy link
Contributor

nre-ableton commented Jun 29, 2020

Also, I couldn't get CodeNarc 1.6 to run on the current master branch of JenkinsPipelineUnit, I get an exception:

ERROR org.codenarc.rule.AbstractRule - Error from [org.codenarc.rule.formatting.IndentationRule] processing source file [./src/main/groovy/com/lesfurets/jenkins/unit/RegressionTest.groovy]
java.lang.NullPointerException
	at java.util.regex.Matcher.getTextLength(Matcher.java:1283)
	at java.util.regex.Matcher.reset(Matcher.java:309)
	at java.util.regex.Matcher.<init>(Matcher.java:229)
	at java.util.regex.Pattern.matcher(Pattern.java:1093)
        ... snip ...

It seems that CodeNarc doesn't like the trait keyword. I'll file a separate issue there and see if this can be sorted out: CodeNarc/CodeNarc#526.

@nre-ableton nre-ableton changed the title JenkinsPipelineUnit fails linting Add CodeNarc linting checks Jul 31, 2020
@nre-ableton
Copy link
Contributor

BTW I have changed the title of this issue, since the previous title makes it seem like some existing check is failing.

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

No branches or pull requests

3 participants