Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ import org.gradle.api.plugins.quality.CheckstylePlugin
import org.gradle.api.plugins.quality.CodeNarc
import org.gradle.api.plugins.quality.CodeNarcExtension
import org.gradle.api.plugins.quality.CodeNarcPlugin
import org.gradle.api.tasks.Copy
import org.gradle.process.ExecSpec
import org.apache.tools.ant.taskdefs.condition.Os

@CompileStatic
class GrailsCodeStylePlugin implements Plugin<Project> {
Expand All @@ -49,6 +52,7 @@ class GrailsCodeStylePlugin implements Plugin<Project> {
void apply(Project project) {
initExtension(project)
configureCodeStyle(project)
registerFormattingTasks(project)
doNotApplyStylingToTests(project)
}

Expand Down Expand Up @@ -108,8 +112,10 @@ class GrailsCodeStylePlugin implements Plugin<Project> {
}

private static void doNotApplyStylingToTests(Project project) {
project.tasks.named('checkstyleTest') {
it.enabled = false // Do not check test sources at this time
if (project.tasks.names.contains('checkstyleTest')) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you help me understand why this change was necessary?

project.tasks.named('checkstyleTest') {
it.enabled = false // Do not check test sources at this time
}
}

project.afterEvaluate {
Expand Down Expand Up @@ -191,4 +197,88 @@ class GrailsCodeStylePlugin implements Plugin<Project> {
)
}
}

private static void registerFormattingTasks(Project project) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code style work is planned to move to it's own repository (eventually). I'm ok with putting the formatting logic adjacent to this plugin, but I don't think it should be in this plugin since it's effectively a root only plugin - the root project won't ever have sources. Let's move this to it's own format plugin?

if (project == project.rootProject) {
project.tasks.register('installGitHooks', Copy) {
it.group = 'verification'
it.description = 'Installs the git pre-commit hook for automatic code formatting'
it.from(project.rootProject.layout.projectDirectory.file('etc/hooks/pre-commit'))
it.into(project.rootProject.layout.projectDirectory.dir('.git/hooks'))
it.fileMode = 0755
}
}

project.tasks.register('formatCode') {
it.group = 'verification'
it.description = 'Formats Java and Groovy source files using the IntelliJ command line formatter'

it.doLast {
String ideaHome = (project.findProperty('idea.home') ?: System.getenv('IDEA_HOME')) as String
String executable = Os.isFamily(Os.FAMILY_WINDOWS) ? 'format.bat' : 'format.sh'
File formatExec = null

if (ideaHome) {
formatExec = new File(ideaHome, "bin/$executable")
} else {
// Try common paths on macOS
if (Os.isFamily(Os.FAMILY_MAC)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should just make it a requirement that idea is on the path and reference the version on the path. IntelliJ has an option to install the command line tools (https://www.jetbrains.com/help/idea/working-with-the-ide-features-from-command-line.html) so let's just reference the help article and not trying to be smart about locating it (anyone using JetBrains toolbox won't be installing it to /Applications or to a specific location)

def commonPaths = [
"/Applications/IntelliJ IDEA.app/Contents/bin/$executable",
"/Applications/IntelliJ IDEA CE.app/Contents/bin/$executable"
]
for (path in commonPaths) {
File f = new File(path)
if (f.exists()) {
formatExec = f
break
}
}
}

if (formatExec == null && !Os.isFamily(Os.FAMILY_WINDOWS)) {
// On Linux/Mac, try to find 'idea' in PATH
try {
def out = new ByteArrayOutputStream()
project.exec { ExecSpec exec ->
exec.commandLine 'which', 'idea'
exec.standardOutput = out
exec.ignoreExitValue = true
}
def path = out.toString().trim()
if (path) {
formatExec = new File(new File(path).parentFile, executable)
}
} catch (Exception ignored) { }
}
}

if (formatExec == null || !formatExec.exists()) {
project.logger.error("IntelliJ formatter executable not found.")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's deprecated to reference project from inside of a task. logger should be available already.

project.logger.error("Please set 'idea.home' property or IDEA_HOME environment variable to your IntelliJ installation directory.")
project.logger.error("Example: ./gradlew formatCode -Pidea.home=/Applications/IntelliJ\\ IDEA.app/Contents")
throw new RuntimeException("IntelliJ formatter executable not found.")
}

def filesToFormat = project.findProperty('formatFiles')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I hadn't considered the useful of having a format task added to the subprojects. I still think we should split the format tasks out from the code style though - otherwise you're trying to apply styling to the root project which we don't want to do.

def settingsFile = project.rootProject.file('.idea/codeStyles/Project.xml')

if (!settingsFile.exists()) {
throw new RuntimeException("IntelliJ code style settings not found at ${settingsFile.absolutePath}")
}

project.exec { ExecSpec exec ->
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Take a look at the publishAllToMavenLocal task for how to do in this a modern Gradle way without referencing project .

exec.commandLine formatExec.absolutePath
exec.args '-s', settingsFile.absolutePath
exec.args '-mask', '*.java,*.groovy'
exec.args '-r'
if (filesToFormat) {
exec.args((filesToFormat.toString()).split(','))
} else {
exec.args project.projectDir.absolutePath
}
}
}
}
}
}
12 changes: 8 additions & 4 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
* limitations under the License.
*/

plugins {
id 'org.apache.grails.gradle.grails-code-style'
}

import java.time.Instant
import java.time.LocalDate
import java.time.ZoneOffset
Expand All @@ -27,10 +31,10 @@ import org.apache.tools.ant.taskdefs.condition.Os
ext {
isReproducibleBuild = System.getenv("SOURCE_DATE_EPOCH") != null
buildInstant = java.util.Optional.ofNullable(System.getenv("SOURCE_DATE_EPOCH"))
.filter(s -> !s.isEmpty())
.map(Long::parseLong)
.map(Instant::ofEpochSecond)
.orElseGet(Instant::now)
.filter { s -> !s.isEmpty() }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please revert these changes.

.map { s -> Long.parseLong(s as String) }
.map { l -> Instant.ofEpochSecond(l as Long) }
.orElseGet { Instant.now() }
formattedBuildDate = System.getenv("SOURCE_DATE_EPOCH") ?
DateTimeFormatter.ISO_INSTANT.format(buildInstant) :
DateTimeFormatter.ISO_DATE.format(LocalDate.ofInstant(buildInstant as Instant, ZoneOffset.UTC))
Expand Down
41 changes: 41 additions & 0 deletions etc/hooks/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#!/bin/bash
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#

set -e

# Get staged files that are Groovy or Java
STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACMR | grep -E '\.(groovy|java)$' || true)

if [ -n "$STAGED_FILES" ]; then
echo "Formatting staged Groovy/Java files using IntelliJ formatter..."

# Convert newline-separated list to comma-separated for Gradle property
FILES_COMMAS=$(echo "$STAGED_FILES" | tr '\n' ',' | sed 's/,$//')

# Run the Gradle formatting task
./gradlew :formatCode -PformatFiles="$FILES_COMMAS"

# Re-stage the files in case they were modified by the formatter
for FILE in $STAGED_FILES; do
if [ -f "$FILE" ]; then
git add "$FILE"
fi
done
fi