[
https://issues.apache.org/jira/browse/GROOVY-12132?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095470#comment-18095470
]
ASF GitHub Bot commented on GROOVY-12132:
-----------------------------------------
paulk-asert merged PR #2667:
URL: https://github.com/apache/groovy/pull/2667
> groovyc: print collected warnings on successful compilation (make -w
> effective)
> -------------------------------------------------------------------------------
>
> Key: GROOVY-12132
> URL: https://issues.apache.org/jira/browse/GROOVY-12132
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
> h2. Code changes
> Two files:
> * {{src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java}} — after
> a successful {{compile()}} in {{doCompilation(...)}}, a new private
> {{displayWarnings(unit)}} writes any collected warnings to {{System.err}} via
> the existing {{ErrorCollector.write(PrintWriter, Janitor)}}, i.e. the
> identical diagnostic format already used when compilation *fails* (message,
> source line, caret, {{"N warning(s)"}} summary). The collector's contents are
> already filtered by the configured warning level at collection time, so no
> additional gating is needed.
> * {{src/test/groovy/org/codehaus/groovy/tools/FileSystemCompilerTest.java}} —
> two tests (written failing-first): a warning injected via a
> {{CompilationCustomizer}} must appear on stderr after a *successful* compile;
> a clean compile must print nothing.
> Behaviour notes:
> * This makes the long-documented {{-w}}/{{--warningLevel}} option effective:
> it is described as "the amount of warnings to *print*", but until now nothing
> ever printed collected warnings on success — the option only changed what was
> collected and then discarded.
> * {{doCompilation}} is also the entry point for Ant's {{<groovyc>}}
> in-process compiles, so Ant builds will start seeing collected warnings on
> stderr.
> * {{GroovyMain}} (the {{groovy}} script runner) has the same
> discard-on-success behaviour and is deliberately *not* changed here —
> printing warnings on every script run is a noisier proposition, worth its own
> discussion.
> * Found in passing, not addressed here: {{groovyc}} constructs {{new
> CompilerConfiguration()}} and therefore ignores {{groovy.warnings}} and the
> other documented {{groovy.*}} system properties, while {{groovy}} honours
> them via {{new CompilerConfiguration(System.getProperties())}}.
> Example ({{@TupleConstructor(force=true)}} generating a duplicate
> constructor; the warning is level 2, so silent at the default {{-w 1}}):
> {noformat}
> $ groovyc -w 2 Dup.groovy
> warning: @groovy.transform.TupleConstructor(force = true)
> ^
> Dup.groovy: 1: @TupleConstructor specifies duplicate constructor:
> P(java.lang.String)
> 1 warning
> {noformat}
> h2. Assessment of current warning usage
> All warnings funnel into {{ErrorCollector.addWarning(...)}}, which filters by
> the configured level *at collection time*. {{SourceUnit.addWarning(text,
> node)}} is a convenience wrapper that hardcodes {{POSSIBLE_ERRORS}} (2), and
> {{ModuleNode.getContext()}} returns the {{SourceUnit}}, so every
> {{getContext().addWarning(...)}} caller inherits level 2 whether it intended
> to or not.
> The inventory is small: ten emission sites in core, none in any subproject,
> none in the parser, and no compile-time deprecation warnings at all.
> {{PARANOIA}} (3) has no emitters. Prior to GROOVY-12131, exactly one stock
> warning was emitted at a level visible by default — and that one passes null
> context/source, so it renders without file/line information.
> || Site || Trigger || Level today || Proposed || Rationale ||
> | {{ClosureSignatureHint.findClassNode}} | {{@ClosureParams}} hint class name
> uses generics — "... doesn't support generics" | 1 | 1 | already correct; but
> passes null context/source (existing TODO) — should gain a proper location
> before warnings become visible |
> | {{OptimizerVisitor}} (GROOVY-12131) | cached constant node shared across
> classes | 1 | 1 | true positive implies miscompilation |
> | {{ModuleNode}} (~789) | multiple valid {{main}} variants; lower-priority
> one unreachable from the Groovy runner | 2 | 1 | a dead entry point is almost
> certainly unintended |
> | {{Verifier:469}} (GROOVY-11508) | *trait* implemented more than once with
> different generic arguments | 2 | 1 | the interface equivalent is a hard
> error; the trait case was only demoted to keep code compiling |
> | {{Verifier:992}} | generated property accessor cannot override a final
> method | 2 | 1 | the accessor is silently not generated as an override —
> surprising behaviour |
> | {{Verifier:1236}} | default-argument expansion collides with an explicit
> constructor | 2 | 1 | a generated-ctor collision is a probable mistake
> (mirrors the {{@TupleConstructor}} case) |
> | {{TupleConstructorASTTransformation:342}} | {{@TupleConstructor}} generates
> a duplicate constructor | 2 | 1 | as above |
> | {{ASTTransformationVisitor:329}} | global transform class declared in two
> descriptor files (first wins) | 2 | 2 | commonly caused by benign duplicate
> jars on messy classpaths; promoting would be noisy |
> | {{ASTTransformationVisitor:340}} | {{URISyntaxException}} comparing those
> descriptor URLs | 2 | 2 | rare internal oddity |
> | {{ASTTransformationVisitor:389}} | declared global transform class lacks
> {{@GroovyASTTransformation}} — "may fail and cause the compilation to fail" |
> 2 | 1 | the message itself says it is a likely error |
> | {{StaticTypeCheckingVisitor:750}} (GROOVY-11360) | field hidden by a
> dynamic property under {{@TypeChecked}} | 2 | 1 | user opted into strictness;
> silent hiding is exactly what they asked to be told about |
> Additional cleanups that pair with recalibration:
> * The three {{ASTTransformationVisitor}} sites and {{ClosureSignatureHint}}
> pass null context and/or null source unit — they predate anyone reading the
> output; their rendering should be checked/fixed since they may be among the
> first warnings users ever see.
> * {{SourceUnit.addWarning(text, node)}} offers no way to choose a level,
> quietly biasing new warnings toward invisibility; consider an overload taking
> an importance.
> h2. Compatibility
> No behavioural change for error-free, warning-free compiles (the common case)
> — stderr stays empty. Builds that treat any stderr output as failure could
> notice newly visible warnings; Groovy has no {{-Werror}}-style flag, so exit
> codes are unaffected. If level recalibration lands with this change, the
> promoted sites above become visible at the default {{-w 1}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
