[
https://issues.apache.org/jira/browse/GROOVY-12013?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18081347#comment-18081347
]
ASF GitHub Bot commented on GROOVY-12013:
-----------------------------------------
Copilot commented on code in PR #2536:
URL: https://github.com/apache/groovy/pull/2536#discussion_r3252231663
##########
subprojects/groovy-typecheckers/src/main/groovy/groovy/typecheckers/CombinerChecker.groovy:
##########
@@ -0,0 +1,344 @@
+/*
+ * 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 groovy.typecheckers
+
+import org.apache.groovy.lang.annotation.Incubating
+import org.apache.groovy.typecheckers.CheckingVisitor
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.CodeVisitorSupport
+import org.codehaus.groovy.ast.MethodNode
+import org.codehaus.groovy.ast.expr.ArgumentListExpression
+import org.codehaus.groovy.ast.expr.BinaryExpression
+import org.codehaus.groovy.ast.expr.ClassExpression
+import org.codehaus.groovy.ast.expr.ClosureExpression
+import org.codehaus.groovy.ast.expr.ConstantExpression
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.expr.MethodCallExpression
+import org.codehaus.groovy.ast.expr.MethodPointerExpression
+import org.codehaus.groovy.ast.expr.VariableExpression
+import org.codehaus.groovy.transform.stc.GroovyTypeCheckingExtensionSupport
+import org.codehaus.groovy.transform.stc.StaticTypesMarker
+
+/**
+ * A compile-time checker that verifies the combiner passed to an
+ * order-independent parallel reduction ({@code sumParallel},
+ * {@code injectParallel}) carries the associativity contract those methods
+ * silently require — mirroring the design of {@code PurityChecker}.
+ * <p>
+ * Associativity is undecidable in general, so this checker is deliberately
+ * conservative. It has two modes, selected via the extension option
+ * {@code mode}:
+ * <pre>
+ * // default (lenient): only flag high-confidence problems
+ * {@code @TypeChecked(extensions = 'groovy.typecheckers.CombinerChecker')}
+ *
+ * // strict: additionally require the combiner to be declared
@Associative/@Reducer
+ * {@code @TypeChecked(extensions = "groovy.typecheckers.CombinerChecker(mode:
'strict')")}
+ * </pre>
+ * <p>
+ * What it does, by combiner shape:
+ * <ul>
+ * <li><b>Method reference</b> to a method annotated {@link
groovy.transform.Associative}
+ * or {@link groovy.transform.Reducer} — accepted.</li>
+ * <li><b>Inline closure</b> whose combine expression applies a
non-associative
+ * operator ({@code -}, {@code /}, {@code %}) directly to the two
combiner
+ * parameters (e.g. <code>{ a, b {@literal ->} a - b }</code>) —
flagged in
+ * <em>any</em> mode (high confidence).</li>
+ * <li><b>{@code injectParallel}</b> seed that contradicts a {@code
@Reducer}'s
+ * declared {@code zero()} (both statically constant) —
flagged.</li>
+ * <li>Un-annotated method references and other inline closures —
flagged
+ * only in <b>strict</b> mode (a declared, verifiable contract is
required).</li>
+ * </ul>
Review Comment:
The Javadoc says lenient mode flags only `-`, `/`, `%` for inline combiners,
but `NON_ASSOCIATIVE_OPS` (and the tests) include `**` as well. Please update
the Javadoc description (and/or the operator set) so the documented behavior
matches what the checker actually enforces.
##########
subprojects/groovy-typecheckers/src/spec/doc/typecheckers.adoc:
##########
@@ -1241,3 +1250,141 @@ Both checkers can be used independently or together:
@TypeChecked(extensions = ['groovy.typecheckers.ModifiesChecker',
'groovy.typecheckers.PurityChecker'])
----
+
+== Checking @Associative/@Reducer Combiners (Incubating)
+
+The parallel `Collection` reductions split, reorder, and recombine their input.
+That is only correct when the combining function is *associative* --
+`combine(a, combine(b, c))` must equal `combine(combine(a, b), c)`
> New optional type checking extension: CombinerChecker to verify associative
> combiners in injectParallel/sumParallel/Stream.reduce
> ---------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12013
> URL: https://issues.apache.org/jira/browse/GROOVY-12013
> Project: Groovy
> Issue Type: New Feature
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> h3. Summary
> Adds two incubating annotations and an opt-in type-checking extension that
> verify, at compile time, that the combining function passed to an
> order-independent reduction is associative — the contract such reductions
> silently require but cannot otherwise enforce.
> h3. Annotations ({{groovy.transform}})
> * {{@Associative}} — declares a two-argument combiner is associative:
> {{combine(a, combine(b, c)) == combine(combine(a, b), c)}}.
> * {{@Reducer}} — associative *and* has an identity element, named via the
> {{zero()}} member (e.g. {{@Reducer(zero = '0')}}).
> h3. CombinerChecker ({{groovy.typecheckers}})
> A type-checking extension enabled with
> {{@TypeChecked(extensions='groovy.typecheckers.CombinerChecker')}}.
> It inspects the combiner of:
> * Groovy's parallel collection reductions {{injectParallel}} and
> {{sumParallel}};
> * the JDK stream reductions {{Stream}}, {{IntStream}}, {{LongStream}} and
> {{DoubleStream}} {{reduce}} (all overloads).
> The combiner is accepted when it is:
> * a method reference to an {{@Associative}}/{{@Reducer}} method; or
> * a method reference to, or a thin delegating closure over, a {{Monoid}}
> or {{Semigroup}} from any library (matched by simple type name —
> Functional Java, Palatable Lambda, Purefun, etc. — configurable via the
> {{monoids}}/{{semigroups}} options, no dependency added).
> It reports a compile-time error when:
> * an inline closure applies a non-associative operator ({{-}}, {{/}}, {{%}},
> {{**}}) directly to its two arguments;
> * the seed of a seeded reduction contradicts a {{@Reducer}}'s declared
> identity;
> * a {{Semigroup}} (which has no identity) is used with a seeded reduction
> that requires one.
> Two modes: the default (lenient) flags only high-confidence problems; *strict*
> ({{CombinerChecker(mode: 'strict')}}) additionally requires the combiner to
> carry a declared, verifiable contract.
> h3. Notes and non-goals
> * The checker runs only under {{@TypeChecked}}/{{@CompileStatic}} and treats
> the annotations/carrier types as asserted contracts (not proofs);
> sequential {{inject}} is deliberately never flagged.
> * The inline-closure check is syntactic and assumes operators carry their
> conventional meaning. Resolving overloaded operators is a non-goal: in
> particular, overloading a normally-associative operator (e.g. {{+}}, {{*}})
> to be non-associative is poor style and is not detected by design. The
> declaration paths ({{@Associative}}/{{@Reducer}}, Monoid/Semigroup) are the
> reliable, overloading-immune way to assert the contract.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
