Copilot commented on code in PR #2450: URL: https://github.com/apache/groovy/pull/2450#discussion_r3055877174
########## build-logic/src/main/groovy/org/apache/groovy/gradle/SwingBuilderWidgetDocTask.groovy: ########## @@ -0,0 +1,85 @@ +/* + * 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 + * + * http://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. + */ +package org.apache.groovy.gradle + +import groovy.transform.CompileDynamic +import groovy.transform.CompileStatic +import org.gradle.api.DefaultTask +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.DirectoryProperty +import org.gradle.api.tasks.CacheableTask +import org.gradle.api.tasks.Classpath +import org.gradle.api.tasks.InputFiles +import org.gradle.api.tasks.OutputDirectory +import org.gradle.api.tasks.TaskAction +import org.gradle.process.ExecOperations + +import javax.inject.Inject + +/** + * Generates AsciiDoc documentation for SwingBuilder widgets by introspecting + * the SwingBuilder class from the project being built. + * + * The generator script is bundled as a resource in the build-logic jar. + * At execution time it is extracted to a temporary file and run via + * {@code groovy.ui.GroovyMain} in a forked JVM whose classpath contains the + * groovy-swing classes being built (plus the core Groovy jar they depend on). + */ +@CacheableTask +@CompileStatic +class SwingBuilderWidgetDocTask extends DefaultTask { + + private static final String SCRIPT_RESOURCE = 'org/apache/groovy/gradle/GenerateSwingBuilderWidgetDocs.groovy' + + private final ExecOperations execOperations + + @InputFiles + @Classpath + final ConfigurableFileCollection classpath = project.objects.fileCollection() + + @OutputDirectory + final DirectoryProperty outputDirectory = project.objects.directoryProperty() + + @Inject + SwingBuilderWidgetDocTask(ExecOperations execOperations) { + this.execOperations = execOperations + description = 'Generates AsciiDoc widget reference for SwingBuilder.' + group = 'documentation' + } + + @TaskAction + @CompileDynamic + void generate() { + outputDirectory.get().asFile.mkdirs() + + // Extract the generator script from build-logic resources to a temp file + File scriptFile = File.createTempFile('GenerateSwingBuilderWidgetDocs', '.groovy') + scriptFile.deleteOnExit() + getClass().classLoader.getResourceAsStream(SCRIPT_RESOURCE).withStream { input -> + scriptFile.bytes = input.bytes + } + + execOperations.javaexec { + it.mainClass.set('groovy.ui.GroovyMain') + it.classpath = this.classpath + it.jvmArgs('-Djava.awt.headless=true') + it.args(scriptFile.absolutePath, outputDirectory.asFile.get().absolutePath) Review Comment: Using `File.createTempFile(...)` with `deleteOnExit()` can leak temp files under the Gradle daemon (the JVM may not exit for a long time). Prefer creating the script under Gradle’s `temporaryDir` and deleting it after execution (or reuse a stable file name in `temporaryDir`). ```suggestion // Extract the generator script from build-logic resources to the task temporary directory temporaryDir.mkdirs() File scriptFile = new File(temporaryDir, 'GenerateSwingBuilderWidgetDocs.groovy') getClass().classLoader.getResourceAsStream(SCRIPT_RESOURCE).withStream { input -> scriptFile.bytes = input.bytes } try { execOperations.javaexec { it.mainClass.set('groovy.ui.GroovyMain') it.classpath = this.classpath it.jvmArgs('-Djava.awt.headless=true') it.args(scriptFile.absolutePath, outputDirectory.asFile.get().absolutePath) } } finally { scriptFile.delete() ``` ########## build-logic/src/main/resources/org/apache/groovy/gradle/GenerateSwingBuilderWidgetDocs.groovy: ########## @@ -0,0 +1,361 @@ +/* + * 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 + * + * http://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. + */ + +/** + * Generates AsciiDoc documentation for SwingBuilder widgets. + * + * This script introspects a live SwingBuilder instance to discover all + * registered nodes, their categories, underlying Swing classes, and + * JavaBeans properties. It produces a single file + * (_swing-builder-widgets.adoc) containing category summary tables + * followed by detail sections for each widget. + * + * Usage: + * groovy GenerateSwingBuilderWidgetDocs.groovy [outputDir] + * + * If outputDir is omitted, output goes to src/spec/doc under this subproject. + */ + +import groovy.swing.SwingBuilder +import groovy.swing.factory.BeanFactory +import groovy.swing.factory.ComponentFactory +import groovy.swing.factory.RichActionWidgetFactory +import groovy.swing.factory.TextArgWidgetFactory +import groovy.swing.factory.FormattedTextFactory +import groovy.swing.factory.LayoutFactory +import groovy.swing.factory.TabbedPaneFactory +import groovy.swing.factory.WidgetFactory Review Comment: These imports appear unused in the script (no references in the file). Removing them will reduce noise and avoid implying behavior that isn’t actually present. ```suggestion ``` ########## build-logic/src/main/resources/org/apache/groovy/gradle/GenerateSwingBuilderWidgetDocs.groovy: ########## @@ -0,0 +1,361 @@ +/* + * 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 + * + * http://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. + */ + +/** + * Generates AsciiDoc documentation for SwingBuilder widgets. + * + * This script introspects a live SwingBuilder instance to discover all + * registered nodes, their categories, underlying Swing classes, and + * JavaBeans properties. It produces a single file + * (_swing-builder-widgets.adoc) containing category summary tables + * followed by detail sections for each widget. + * + * Usage: + * groovy GenerateSwingBuilderWidgetDocs.groovy [outputDir] + * + * If outputDir is omitted, output goes to src/spec/doc under this subproject. + */ + +import groovy.swing.SwingBuilder +import groovy.swing.factory.BeanFactory +import groovy.swing.factory.ComponentFactory +import groovy.swing.factory.RichActionWidgetFactory +import groovy.swing.factory.TextArgWidgetFactory +import groovy.swing.factory.FormattedTextFactory +import groovy.swing.factory.LayoutFactory +import groovy.swing.factory.TabbedPaneFactory +import groovy.swing.factory.WidgetFactory + +import java.beans.BeanInfo +import java.beans.Introspector +import java.beans.PropertyDescriptor + +// Headless mode should be set via -Djava.awt.headless=true on the JVM command line +if (!Boolean.getBoolean('java.awt.headless')) { + System.setProperty('java.awt.headless', 'true') +} + +def outputDir = args.length > 0 ? new File(args[0]) : new File('src/spec/doc') +outputDir.mkdirs() + +def builder = new SwingBuilder() + +// Collect all factories keyed by node name +Map<String, groovy.util.Factory> factories = builder.factories + +// Collect registration groups (categories) - these come from the register*() method names +Map<String, Set<String>> groups = [:] +builder.registrationGroups.each { groupName -> + def items = builder.getRegistrationGroupItems(groupName) + if (items && groupName) { + groups[groupName] = items + } +} + +// Also collect explicit methods (edt, doLater, etc.) +Map<String, Closure> explicitMethods = builder.localExplicitMethods + +// Human-friendly category names derived from method name suffixes +def categoryLabel = { String groupName -> + // e.g. "ActionButtonWidgets" -> "Action Button Widgets" + groupName.replaceAll(/([a-z])([A-Z])/, '$1 $2') Review Comment: `categoryLabel` only inserts spaces between a lowercase letter followed by an uppercase letter, which leaves acronym-prefixed groups like `MDIWidgets` unformatted in the generated docs (currently rendered as “MDIWidgets”). Consider enhancing the splitting logic to also handle acronym boundaries so headings are consistently human-readable. ```suggestion // and "MDIWidgets" -> "MDI Widgets" groupName .replaceAll(/([A-Z]+)([A-Z][a-z])/, '$1 $2') .replaceAll(/([a-z])([A-Z])/, '$1 $2') ``` ########## build-logic/src/main/resources/org/apache/groovy/gradle/GenerateSwingBuilderWidgetDocs.groovy: ########## @@ -0,0 +1,361 @@ +/* + * 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 + * + * http://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. + */ + +/** + * Generates AsciiDoc documentation for SwingBuilder widgets. + * + * This script introspects a live SwingBuilder instance to discover all + * registered nodes, their categories, underlying Swing classes, and + * JavaBeans properties. It produces a single file + * (_swing-builder-widgets.adoc) containing category summary tables + * followed by detail sections for each widget. + * + * Usage: + * groovy GenerateSwingBuilderWidgetDocs.groovy [outputDir] + * + * If outputDir is omitted, output goes to src/spec/doc under this subproject. + */ + +import groovy.swing.SwingBuilder +import groovy.swing.factory.BeanFactory +import groovy.swing.factory.ComponentFactory +import groovy.swing.factory.RichActionWidgetFactory +import groovy.swing.factory.TextArgWidgetFactory +import groovy.swing.factory.FormattedTextFactory +import groovy.swing.factory.LayoutFactory +import groovy.swing.factory.TabbedPaneFactory +import groovy.swing.factory.WidgetFactory + +import java.beans.BeanInfo +import java.beans.Introspector +import java.beans.PropertyDescriptor + +// Headless mode should be set via -Djava.awt.headless=true on the JVM command line +if (!Boolean.getBoolean('java.awt.headless')) { + System.setProperty('java.awt.headless', 'true') +} + +def outputDir = args.length > 0 ? new File(args[0]) : new File('src/spec/doc') +outputDir.mkdirs() + +def builder = new SwingBuilder() + +// Collect all factories keyed by node name +Map<String, groovy.util.Factory> factories = builder.factories + +// Collect registration groups (categories) - these come from the register*() method names +Map<String, Set<String>> groups = [:] +builder.registrationGroups.each { groupName -> + def items = builder.getRegistrationGroupItems(groupName) + if (items && groupName) { + groups[groupName] = items + } +} + +// Also collect explicit methods (edt, doLater, etc.) +Map<String, Closure> explicitMethods = builder.localExplicitMethods + +// Human-friendly category names derived from method name suffixes +def categoryLabel = { String groupName -> + // e.g. "ActionButtonWidgets" -> "Action Button Widgets" + groupName.replaceAll(/([a-z])([A-Z])/, '$1 $2') +} + +/** + * Known Swing class mappings for factories that don't expose the class via a standard field. + * This covers factories extending AbstractFactory directly (e.g. RootPaneContainerFactory subclasses) + * and custom factories with non-standard constructors. + */ +def knownSwingClasses = [ + 'frame' : javax.swing.JFrame, + 'dialog' : javax.swing.JDialog, + 'window' : javax.swing.JWindow, + 'internalFrame' : javax.swing.JInternalFrame, + 'action' : javax.swing.Action, + 'imageIcon' : javax.swing.ImageIcon, + 'comboBox' : javax.swing.JComboBox, + 'list' : javax.swing.JList, + 'separator' : javax.swing.JSeparator, + 'scrollPane' : javax.swing.JScrollPane, + 'splitPane' : javax.swing.JSplitPane, + 'table' : javax.swing.JTable, + 'formattedTextField': javax.swing.JFormattedTextField, + 'box' : javax.swing.Box, + 'hbox' : javax.swing.Box, + 'vbox' : javax.swing.Box, +] + +/** + * Determine the underlying Swing/AWT class for a given factory. + */ +def resolveSwingClass = { String nodeName, groovy.util.Factory factory -> + if (factory == null) return null + + // Check known mappings first + if (knownSwingClasses.containsKey(nodeName)) { + return knownSwingClasses[nodeName] + } + + // BeanFactory and subclasses (ComponentFactory, TabbedPaneFactory, etc.) store beanClass + if (factory instanceof BeanFactory) { + return factory.beanClass + } + // RichActionWidgetFactory stores klass + if (factory instanceof RichActionWidgetFactory) { + return factory.klass + } + // TextArgWidgetFactory extends RichActionWidgetFactory + if (factory instanceof TextArgWidgetFactory) { + return factory.klass + } Review Comment: `TextArgWidgetFactory` extends `RichActionWidgetFactory`, so the `instanceof TextArgWidgetFactory` branch is unreachable because the `RichActionWidgetFactory` check above will already match. Remove the redundant branch or check `TextArgWidgetFactory` first if it needs special handling. ```suggestion // RichActionWidgetFactory and subclasses (including TextArgWidgetFactory) store klass if (factory instanceof RichActionWidgetFactory) { return factory.klass } ``` ########## build-logic/src/main/groovy/org/apache/groovy/gradle/SwingBuilderWidgetDocTask.groovy: ########## @@ -0,0 +1,85 @@ +/* + * 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 + * + * http://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. + */ +package org.apache.groovy.gradle + +import groovy.transform.CompileDynamic +import groovy.transform.CompileStatic +import org.gradle.api.DefaultTask +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.DirectoryProperty +import org.gradle.api.tasks.CacheableTask +import org.gradle.api.tasks.Classpath +import org.gradle.api.tasks.InputFiles +import org.gradle.api.tasks.OutputDirectory +import org.gradle.api.tasks.TaskAction +import org.gradle.process.ExecOperations + +import javax.inject.Inject + +/** + * Generates AsciiDoc documentation for SwingBuilder widgets by introspecting + * the SwingBuilder class from the project being built. + * + * The generator script is bundled as a resource in the build-logic jar. + * At execution time it is extracted to a temporary file and run via + * {@code groovy.ui.GroovyMain} in a forked JVM whose classpath contains the + * groovy-swing classes being built (plus the core Groovy jar they depend on). + */ +@CacheableTask +@CompileStatic +class SwingBuilderWidgetDocTask extends DefaultTask { + + private static final String SCRIPT_RESOURCE = 'org/apache/groovy/gradle/GenerateSwingBuilderWidgetDocs.groovy' + + private final ExecOperations execOperations + + @InputFiles + @Classpath + final ConfigurableFileCollection classpath = project.objects.fileCollection() + + @OutputDirectory + final DirectoryProperty outputDirectory = project.objects.directoryProperty() + + @Inject + SwingBuilderWidgetDocTask(ExecOperations execOperations) { + this.execOperations = execOperations + description = 'Generates AsciiDoc widget reference for SwingBuilder.' + group = 'documentation' + } + + @TaskAction + @CompileDynamic + void generate() { + outputDirectory.get().asFile.mkdirs() + + // Extract the generator script from build-logic resources to a temp file + File scriptFile = File.createTempFile('GenerateSwingBuilderWidgetDocs', '.groovy') + scriptFile.deleteOnExit() + getClass().classLoader.getResourceAsStream(SCRIPT_RESOURCE).withStream { input -> Review Comment: `getResourceAsStream(SCRIPT_RESOURCE)` can return null (e.g., if the resource isn’t packaged as expected), and calling `withStream` on null will throw an unhelpful NPE. Please add an explicit null check and fail with a clear exception message if the generator script resource cannot be found. ```suggestion def scriptResourceStream = getClass().classLoader.getResourceAsStream(SCRIPT_RESOURCE) if (scriptResourceStream == null) { throw new IllegalStateException("Generator script resource not found: ${SCRIPT_RESOURCE}") } scriptResourceStream.withStream { input -> ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
