Skip to content

WIP: Enforce Code Style #14925

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

Draft
wants to merge 20 commits into
base: 7.0.x
Choose a base branch
from
Draft

WIP: Enforce Code Style #14925

wants to merge 20 commits into from

Conversation

matrei
Copy link
Contributor

@matrei matrei commented Jul 22, 2025

Rules can be introduced gradually, and source files reformatted incrementally to maintain a manageable review process and avoid overwhelming diffs.

Formatting commits will be squashed before merging to minimize noise in the project history.

Related: #14903

matrei added 5 commits July 22, 2025 14:15
Add Checkstyle and Codenarc configuration for the `grails.core.ROOT`
and `grails-gradle` projects to enforce consistent coding standards.

Rules will be introduced gradually, and source files reformatted
incrementally to maintain a manageable review process and avoid
overwhelming diffs.

Formatting commits will be squashed before merging to minimize noise in
the project history.
Add the `NewlineAtEndOfFile` Checkstyle rule and
`FileEndsWithoutNewline` Codenarc rule to ensure all source files end
with a single newline character.

This is a long-standing POSIX convention and is assumed by many Unix
tools. It also helps avoid unnecessary noise in version control diffs
and ensures consistent formatting across editors and operating systems.

Most modern IDEs can be configured to add the final newline
automatically.

https://checkstyle.sourceforge.io/checks/misc/newlineatendoffile.html
https://codenarc.org/codenarc-rules-formatting.html#fileendswithoutnewline-rule
Add the `FileTabCharacter` Checkstyle rule and
`NoTabCharacter` Codenarc rule to ensure that source files
use spaces instead of tabs.

This helps maintain consistent indentation and avoids
formatting issues across different editors and environments.

https://checkstyle.sourceforge.io/checks/whitespace/filetabcharacter.html
https://codenarc.org/codenarc-rules-convention.html#notabcharacter-rule
Allows selective suppression of specific Checkstyle rules where
necessary, improving flexibility for edge cases or legacy code.
Add the Checkstyle `UnusedImports` rule and the Codenarc
`UnusedImport` and `UnnecessaryGroovyImport` rules.

These checks help clean up forgotten or unnecessary import statements,
keeping the codebase tidy and easier to maintain.

https://checkstyle.sourceforge.io/checks/imports/unusedimports.html#UnusedImports
https://codenarc.org/codenarc-rules-imports.html#unusedimport-rule
https://codenarc.org/codenarc-rules-imports.html#unnecessarygroovyimport-rule
@@ -62,6 +62,10 @@ micronautVersion=4.6.5
micronautSerdeJacksonVersion=2.11.0
grailsSpringSecurityVersion=7.0.0-SNAPSHOT

# build dependencies for code quality checks
checkstyleVersion=10.26.1
codenarcVersion=3.6.0
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a Groovy 4 version 3.6.0-groovy-4.0, which I think we should consider using. https://github.com/CodeNarc/CodeNarc/blob/master/CHANGELOG.md

matrei added 5 commits July 23, 2025 11:07
Order imports alphabetically within these groups:

  1) java
  2) javax
  3) groovy (groovy, org.apache.groovy, org.codehaus.groovy)
  4) jakarta
  5) all others
  6) org.springframework
  7) grails (grails, org.apache.grails, org.grails)
  8) all static imports

This mirrors Spring Framework’s convention and keeps imports tidy,
predictable, and easy to scan.

Checkstyle can enforce this for Java. I couldn’t find a way to make
CodeNarc or Spotless apply the same grouping for Groovy.

Most IDEs, including IntelliJ, can apply this import ordering and
grouping automatically, either on save or via a shortcut, when
properly configured. The code style settings can be shared in the
repository to ensure consistency.
# Conflicts:
#	grails-common/src/main/groovy/org/apache/grails/common/annotation/RecursiveAnnotationAttributesVisitor.java
#	grails-common/src/main/groovy/org/apache/grails/common/compiler/asm/AnnotationMetadataReader.java
#	grails-common/src/main/groovy/org/apache/grails/common/compiler/asm/ClassReader.java
#	grails-common/src/main/groovy/org/apache/grails/common/compiler/asm/Context.java
#	grails-core/src/main/groovy/grails/boot/config/tools/ClassPathScanner.groovy
#	grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/utils/AnnotationMetadataReaderFactory.java
matrei added 4 commits August 8, 2025 15:57
Description from Codenarc rule `UnnecessaryGString`:
String objects should be created with single quotes, and GString objects
created with double quotes. Creating normal String objects with double
quotes is confusing to readers.
# Conflicts:
#	grails-gradle/docs-core/src/main/groovy/grails/doc/PdfBuilder.groovy
#	grails-gradle/docs-core/src/main/groovy/grails/doc/gradle/PublishPdf.groovy
#	grails-gsp/core/src/main/groovy/org/grails/gsp/compiler/GroovyPageCompiler.groovy
#	grails-test-examples/app1/grails-app/controllers/functionaltests/commandobjects/CommandObjectController.groovy
These were star-imported and we do not do that anymore.
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.

Code Standards, Reformat and Verification
2 participants