diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 07e922619..69e62f186 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -1,8 +1,27 @@ name: verify on: pull_request: + +# Code Scanning needs write access to upload SARIF results for inline annotations. +permissions: + contents: read + security-events: write + jobs: - pmd: + # Fast, early-fail lint lane: PMD + Checkstyle (+ CPD). Turns red in a few + # minutes on any violation, independent of the long build below, so a stray + # PMD/Checkstyle issue is reported immediately rather than after `verify`. + # + # `compile` is in the same invocation as the analysis goals: PMD's + # type-resolving rules (e.g. InvalidLogMessageFormat on the SLF4J + # trailing-Throwable idiom) need Tycho's aux-classpath, which a fresh `mvn` + # does not inherit from a prior step's target/classes. + # + # `--fail-never` lets every module produce its report (no Maven cascade-skip), + # so the uploaded SARIF — and therefore the inline annotations — are complete. + # The gate is a jq count over the merged SARIFs; compile errors still halt + # (MojoExecutionException is not suppressed by --fail-never). + lint: runs-on: ubuntu-24.04 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -12,10 +31,90 @@ jobs: java-version: '21' - name: Set up Workspace Environment Variable run: echo "WORKSPACE=${{ github.workspace }}" >> $GITHUB_ENV - - name: PMD Check - run: mvn pmd:pmd pmd:cpd pmd:check pmd:cpd-check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end - checkstyle: + - name: Cache Maven dependencies + uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5 + with: + path: /home/runner/.m2/repository + key: ${{ runner.os }}-maven-0-${{ hashFiles('**/pom.xml') }} + restore-keys: ${{ runner.os }}-maven-0- + + - name: PMD + Checkstyle reports (SARIF) + # PMD: SarifRenderer FQCN — emits pmd.sarif.json AND keeps pmd.xml. + # Checkstyle: output.format=sarif — SARIF content in checkstyle-result.xml. + # CPD is excluded here: the global -Dformat flag uses PMD's Renderer + # hierarchy and would ClassCastException CPD's CPDReportRenderer. + run: | + mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \ + compile \ + pmd:pmd checkstyle:checkstyle \ + -Dformat=net.sourceforge.pmd.renderers.SarifRenderer \ + -Dcheckstyle.output.format=sarif + + - name: CPD report (separate invocation — no SARIF support) + # CPD has no SARIF renderer; emits cpd.xml only. Run standalone so the + # PMD -Dformat flag isn't in scope. + # NOTE: project CPD token threshold is currently very high (issue #1339), + # which effectively disables detection; re-tune once #1339 lands. + run: | + mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \ + compile \ + pmd:cpd-check + + - name: Merge per-module SARIFs (PMD + Checkstyle) + if: always() + # Code Scanning accepts one run per category per upload; each analyzer + # writes one SARIF per module, so concatenate each analyzer's results + # into a single run under .sarif-merged/. + run: | + mkdir -p .sarif-merged + # Merge per-module SARIFs into one run. Filter to JSON-parseable files: + # the ddk-parent aggregator writes a plain-XML checkstyle-result.xml that + # would break jq, and modules with no findings may emit non-SARIF stubs. + merge() { # $1 = find-glob, $2 = output + local f valid=() + while IFS= read -r f; do + jq -e . "$f" >/dev/null 2>&1 && valid+=("$f") + done < <(find . -path "$1") + if [ ${#valid[@]} -gt 0 ]; then + jq -s '{ + "$schema": .[0]."$schema", version: .[0].version, + runs: [{ tool: .[0].runs[0].tool, + results: [.[].runs[].results[]?], + invocations: [.[].runs[].invocations[]?] }] + }' "${valid[@]}" > "$2" + fi + } + merge '*/target/pmd.sarif.json' .sarif-merged/pmd.sarif + merge '*/target/checkstyle-result.xml' .sarif-merged/checkstyle.sarif + + - name: Gate on PMD / CPD / Checkstyle violations + run: | + set -eu + sarif_total=$(jq '[.runs[].results[]?] | length' \ + .sarif-merged/pmd.sarif .sarif-merged/checkstyle.sarif 2>/dev/null \ + | awk '{s+=$1} END {print s+0}') + cpd_total=$(find . -name 'cpd.xml' -path '*/target/*' -exec grep -c '/dev/null \ + | awk -F: '{s+=$2} END {print s+0}') + echo "PMD/Checkstyle SARIF violations: $sarif_total" + echo "CPD duplications: $cpd_total" + if [ "$sarif_total" != "0" ] || [ "$cpd_total" != "0" ]; then + echo "::error::Static analysis found violations (PMD/CPD/Checkstyle)." + exit 1 + fi + + - name: Upload PMD/Checkstyle SARIF to Code Scanning + if: always() + uses: github/codeql-action/upload-sarif@7fd177fa680c9881b53cdab4d346d32574c9f7f4 # v3.35.4 + with: + sarif_file: .sarif-merged + category: lint + + # SpotBugs is the slow critical-path analysis (the experiments' durable + # finding), so it runs in its own parallel lane and never delays `lint`. + spotbugs: runs-on: ubuntu-24.04 + env: + MAVEN_OPTS: -Xmx4g steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5 @@ -24,14 +123,66 @@ jobs: java-version: '21' - name: Set up Workspace Environment Variable run: echo "WORKSPACE=${{ github.workspace }}" >> $GITHUB_ENV - - name: Checkstyle Check - run: mvn checkstyle:checkstyle checkstyle:check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end + - name: Cache Maven dependencies + uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5 + with: + path: /home/runner/.m2/repository + key: ${{ runner.os }}-maven-0-${{ hashFiles('**/pom.xml') }} + restore-keys: ${{ runner.os }}-maven-0- + + - name: SpotBugs report (SARIF) + # sarifOutput=true emits spotbugsSarif.json (also writes spotbugsXml.xml). + run: | + mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \ + compile \ + spotbugs:spotbugs \ + -Dspotbugs.sarifOutput=true + + - name: Merge per-module SpotBugs SARIFs + if: always() + run: | + mkdir -p .sarif-merged + valid=() + while IFS= read -r f; do + jq -e . "$f" >/dev/null 2>&1 && valid+=("$f") + done < <(find . -path '*/target/spotbugsSarif.json') + if [ ${#valid[@]} -gt 0 ]; then + jq -s '{ + "$schema": .[0]."$schema", version: .[0].version, + runs: [{ tool: .[0].runs[0].tool, + results: [.[].runs[].results[]?], + invocations: [.[].runs[].invocations[]?] }] + }' "${valid[@]}" > .sarif-merged/spotbugs.sarif + fi + + - name: Gate on SpotBugs violations + run: | + set -eu + sb_total=$(jq '[.runs[].results[]?] | length' .sarif-merged/spotbugs.sarif 2>/dev/null || echo 0) + echo "SpotBugs SARIF violations: $sb_total" + if [ "$sb_total" != "0" ]; then + echo "::error::SpotBugs found violations." + exit 1 + fi + + - name: Upload SpotBugs SARIF to Code Scanning + if: always() + uses: github/codeql-action/upload-sarif@7fd177fa680c9881b53cdab4d346d32574c9f7f4 # v3.35.4 + with: + sarif_file: .sarif-merged + category: spotbugs + line-endings: runs-on: ubuntu-24.04 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - name: Check LF line endings run: bash .github/scripts/check-line-endings.sh + + # Build + tests only. Static analysis now lives in the `lint` and `spotbugs` + # jobs, so the redundant checkstyle/pmd/spotbugs goals are dropped from here — + # this is the wall-clock long pole and no longer re-runs analysis. + # No `-T 2C`: tests are not known to pass reliably under reactor parallelism. maven-verify: runs-on: ubuntu-24.04 steps: @@ -51,9 +202,7 @@ jobs: path: /home/runner/.m2/repository key: ${{ runner.os }}-maven-0-${{ hashFiles('**/pom.xml') }} - name: Build with Maven within a virtual X Server Environment - # Run pmd:pmd and pmd:cpd first to generate reports for all modules, then run pmd:check and pmd:cpd-check - # This ensures all violations are collected and reported before the build fails - run: xvfb-run mvn clean verify checkstyle:check pmd:pmd pmd:cpd pmd:check pmd:cpd-check spotbugs:check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end + run: xvfb-run mvn clean verify -f ./ddk-parent/pom.xml --batch-mode --fail-at-end - name: Fail on missing surefire reports if: always() run: bash .github/scripts/check-surefire-reports.sh diff --git a/com.avaloq.tools.ddk.check.runtime.core/src/com/avaloq/tools/ddk/check/runtime/issue/DispatchingCheckImpl.java b/com.avaloq.tools.ddk.check.runtime.core/src/com/avaloq/tools/ddk/check/runtime/issue/DispatchingCheckImpl.java index ada56e594..59ebacf99 100644 --- a/com.avaloq.tools.ddk.check.runtime.core/src/com/avaloq/tools/ddk/check/runtime/issue/DispatchingCheckImpl.java +++ b/com.avaloq.tools.ddk.check.runtime.core/src/com/avaloq/tools/ddk/check/runtime/issue/DispatchingCheckImpl.java @@ -35,6 +35,7 @@ @SuppressWarnings({"checkstyle:AbstractClassName"}) public abstract class DispatchingCheckImpl extends AbstractCheckImpl { + // CPD-OFF — incidental token overlap (DI field + trace/try idiom), not an extractable unit (#1339) @Inject private ITraceSet traceSet; @@ -72,6 +73,7 @@ public boolean validate(final EClass eClass, final EObject object, final Diagnos State state = new State(); state.chain = diagnostics; + // CPD-ON state.eventCollector = eventCollector; validate(checkMode, object, state); @@ -123,6 +125,7 @@ protected void validate(final String contextName, final String qContextName, fin if (!disabledMethodTracker.isDisabled(contextName)) { Collector eventCollector = diagnosticCollector.getEventCollector(); try { + // CPD-OFF — incidental token overlap (DI field + trace/try idiom), not an extractable unit (#1339) traceStart(qContextName, object, eventCollector); checkAction.run(); } catch (Exception e) { @@ -160,6 +163,7 @@ protected static class State implements ValidationMessageAcceptorMixin, Diagnost // CHECKSTYLE:OFF public DiagnosticChain chain; public CheckType currentCheckType; + // CPD-ON public boolean hasErrors; public ResourceValidationRuleSummaryEvent.Collector eventCollector; // CHECKSTYLE:ON diff --git a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/AbstractCheckDocumentationExtensionHelper.java b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/AbstractCheckDocumentationExtensionHelper.java index cb48bd700..3ef2e3c6e 100644 --- a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/AbstractCheckDocumentationExtensionHelper.java +++ b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/AbstractCheckDocumentationExtensionHelper.java @@ -44,4 +44,18 @@ protected boolean isExtensionEnabled(final IPluginModelBase base, final CheckCat return !config.isGenerateLanguageInternalChecks(); } + /** + * Documentation extensions do not reference a generated target class, so documentation helpers have no target class name and + * provide their own {@link #isExtensionUpdateRequired} logic that never consults it. + * + * @param catalog + * the check catalog + * @return never returns normally + */ + @SuppressWarnings("PMD.UnusedFormalParameter") + @Override + protected String getTargetClassName(final CheckCatalog catalog) { + throw new UnsupportedOperationException("Documentation extension helpers have no target class"); //$NON-NLS-1$ + } + } diff --git a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/AbstractCheckExtensionHelper.java b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/AbstractCheckExtensionHelper.java index bdbab2782..ab98d01b5 100644 --- a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/AbstractCheckExtensionHelper.java +++ b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/AbstractCheckExtensionHelper.java @@ -147,6 +147,41 @@ protected boolean isExtensionUpdateRequired(final CheckCatalog catalog, final IP return extension.getPoint().equals(getExtensionPointId()); // if points are different, given extension must not be updated } + /** + * Checks whether a class-referencing extension (validator / quickfix) needs updating: it must point at this helper's + * extension point and reference exactly one element whose target class, language and catalog name still match the + * check catalog. Shared by the validator and quickfix helpers, whose only difference is {@link #getTargetClassName}. + * + * @param catalog + * the catalog + * @param extension + * the extension + * @param elements + * the elements + * @return true, if the extension must be regenerated + */ + protected boolean isTargetClassExtensionUpdateRequired(final CheckCatalog catalog, final IPluginExtension extension, final Iterable elements) { + // CHECKSTYLE:OFF + // @Format-Off + return getExtensionPointId().equals(extension.getPoint()) + && (!extensionNameMatches(extension, catalog) + || Iterables.size(elements) != 1 + || !targetClassMatches(Iterables.get(elements, 0), getTargetClassName(catalog)) + || catalog.getGrammar() == null && Iterables.get(elements, 0).getAttribute(LANGUAGE_ELEMENT_TAG) != null + || catalog.getGrammar() != null && !languageNameMatches(Iterables.get(elements, 0), catalog.getGrammar().getName())); + // @Format-On + // CHECKSTYLE:ON + } + + /** + * Gets the target class name based on the package path of given check catalog. + * + * @param catalog + * the check catalog + * @return the target class FQN + */ + protected abstract String getTargetClassName(CheckCatalog catalog); + /** * Updates a given extension to values calculated using given check catalog. * diff --git a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckPreferencesExtensionHelper.java b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckPreferencesExtensionHelper.java index 01935d770..fc54260f5 100644 --- a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckPreferencesExtensionHelper.java +++ b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckPreferencesExtensionHelper.java @@ -160,7 +160,8 @@ public String getExtensionPointName(final CheckCatalog catalog) { * the check catalog * @return the target class FQN */ - private String getTargetClassName(final CheckCatalog catalog) { + @Override + protected String getTargetClassName(final CheckCatalog catalog) { return getFromServiceProvider(CheckGeneratorNaming.class, catalog).qualifiedPreferenceInitializerClassName(catalog); } diff --git a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckQuickfixExtensionHelper.java b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckQuickfixExtensionHelper.java index ddb5a3e49..a80a43ec8 100644 --- a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckQuickfixExtensionHelper.java +++ b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckQuickfixExtensionHelper.java @@ -120,22 +120,14 @@ protected void doUpdateExtension(final CheckCatalog catalog, final IPluginExtens * the check catalog * @return the target class FQN */ - private String getTargetClassName(final CheckCatalog catalog) { + @Override + protected String getTargetClassName(final CheckCatalog catalog) { return getFromServiceProvider(CheckGeneratorNaming.class, catalog).qualifiedQuickfixClassName(catalog); } @Override - public boolean isExtensionUpdateRequired(final CheckCatalog catalog, final IPluginExtension extension, final Iterable elements) { - // CHECKSTYLE:OFF - // @Format-Off - return QUICKFIX_EXTENSION_POINT_ID.equals(extension.getPoint()) - && (!extensionNameMatches(extension, catalog) - || Iterables.size(elements) != 1 - || !targetClassMatches(Iterables.get(elements, 0), getTargetClassName(catalog)) - || catalog.getGrammar() == null && Iterables.get(elements, 0).getAttribute(LANGUAGE_ELEMENT_TAG) != null - || catalog.getGrammar() != null && !languageNameMatches(Iterables.get(elements, 0), catalog.getGrammar().getName())); - // @Format-On - // CHECKSTYLE:ON + protected boolean isExtensionUpdateRequired(final CheckCatalog catalog, final IPluginExtension extension, final Iterable elements) { + return isTargetClassExtensionUpdateRequired(catalog, extension, elements); } } diff --git a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckValidatorExtensionHelper.java b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckValidatorExtensionHelper.java index 95fea23bb..6b27ee359 100644 --- a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckValidatorExtensionHelper.java +++ b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckValidatorExtensionHelper.java @@ -112,23 +112,14 @@ private String getCatalogResourceName(final CheckCatalog catalog) { * the check catalog * @return the target class FQN */ - private String getTargetClassName(final CheckCatalog catalog) { + @Override + protected String getTargetClassName(final CheckCatalog catalog) { return getFromServiceProvider(CheckGeneratorNaming.class, catalog).qualifiedValidatorClassName(catalog); } @Override - public boolean isExtensionUpdateRequired(final CheckCatalog catalog, final IPluginExtension extension, final Iterable elements) { - // CHECKSTYLE:OFF - // @Format-Off - return CHECK_EXTENSION_POINT_ID.equals(extension.getPoint()) - && (!extensionNameMatches(extension, catalog) - || Iterables.size(elements) != 1 - || !targetClassMatches(Iterables.get(elements, 0), getTargetClassName(catalog)) - || catalog.getGrammar() == null && Iterables.get(elements, 0).getAttribute(LANGUAGE_ELEMENT_TAG) != null - || catalog.getGrammar() != null && !languageNameMatches(Iterables.get(elements, 0), catalog.getGrammar().getName()) - ); - // @Format-On - // CHECKSTYLE:ON + protected boolean isExtensionUpdateRequired(final CheckCatalog catalog, final IPluginExtension extension, final Iterable elements) { + return isTargetClassExtensionUpdateRequired(catalog, extension, elements); } } diff --git a/com.avaloq.tools.ddk.typesystem.test/src/com/avaloq/tools/ddk/typesystem/ParameterListMatcherTest.java b/com.avaloq.tools.ddk.typesystem.test/src/com/avaloq/tools/ddk/typesystem/ParameterListMatcherTest.java index 84d22023b..6f3946ace 100644 --- a/com.avaloq.tools.ddk.typesystem.test/src/com/avaloq/tools/ddk/typesystem/ParameterListMatcherTest.java +++ b/com.avaloq.tools.ddk.typesystem.test/src/com/avaloq/tools/ddk/typesystem/ParameterListMatcherTest.java @@ -893,6 +893,7 @@ void testUnnamedFormalAfterNamed4() { assertSame(unnamedFormal2, matchResult.getUnnamedFormalsAfterNamed().get(1), UNNAMED_FORMAL_AFTER_NAMED_NOT_LOCATED); } + // CPD-OFF — explicit parameterized test cases, kept readable over shared (#1339) @Test void testForceMatchByPosition1() { List formals = new ArrayList(); @@ -946,5 +947,6 @@ void testForceMatchByPosition3() { checkParameterMatch(IParameterMatchChecker.MatchStatus.MATCH, actuals.get(1), formals.get(1), matches.get(1)); checkParameterMatch(IParameterMatchChecker.MatchStatus.MATCH, actuals.get(2), formals.get(2), matches.get(2)); } + // CPD-ON } diff --git a/com.avaloq.tools.ddk.xtext.test.core/src/com/avaloq/tools/ddk/xtext/test/jupiter/AbstractValidationTest.java b/com.avaloq.tools.ddk.xtext.test.core/src/com/avaloq/tools/ddk/xtext/test/jupiter/AbstractValidationTest.java index 1f103f0d1..412a1a400 100644 --- a/com.avaloq.tools.ddk.xtext.test.core/src/com/avaloq/tools/ddk/xtext/test/jupiter/AbstractValidationTest.java +++ b/com.avaloq.tools.ddk.xtext.test.core/src/com/avaloq/tools/ddk/xtext/test/jupiter/AbstractValidationTest.java @@ -377,6 +377,7 @@ public void apply(final EObject root, final Integer pos) { * actual message */ private void createErrorMessage(final Integer pos, final List diagnosticsOnTargetPosition, final boolean issueFound, final boolean expectedSeverityMatches, final int actualSeverity, final boolean expectedMessageMatches, final String actualMessage) { + // CPD-OFF — test scaffolding; per-test clarity beats extraction (#1339) StringBuilder errorMessage = new StringBuilder(200); if (issueMustBeFound && !issueFound) { errorMessage.append("Expected issue not found. Code '").append(issueCode).append('\n'); @@ -398,6 +399,7 @@ private void createErrorMessage(final Integer pos, final List linkingErrors = object.eResource().getErrors().stream().filter(error -> error instanceof XtextLinkingDiagnostic).collect(Collectors.toList()); final List errorMessages = Lists.transform(linkingErrors, Resource.Diagnostic::getMessage); for (final String referenceName : referenceNames) { @@ -1012,6 +1015,7 @@ public static void assertLinkingErrorsOnResourceExist(final EObject object, fina break; } } + // CPD-ON assertTrue(found, NLS.bind("Expected linking error on \"{0}\" but could not find it", referenceName)); } } diff --git a/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/AbstractFingerprintComputer.java b/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/AbstractFingerprintComputer.java index 3c6aefbf5..fbed766c6 100644 --- a/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/AbstractFingerprintComputer.java +++ b/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/AbstractFingerprintComputer.java @@ -14,19 +14,15 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.Collections; -import java.util.Iterator; import java.util.List; -import org.eclipse.emf.common.util.EList; import org.eclipse.emf.common.util.URI; import org.eclipse.emf.ecore.EAttribute; import org.eclipse.emf.ecore.EObject; import org.eclipse.emf.ecore.EReference; -import org.eclipse.emf.ecore.EStructuralFeature; import org.eclipse.emf.ecore.InternalEObject; import org.eclipse.emf.ecore.resource.Resource; import org.eclipse.emf.ecore.util.EcoreUtil; -import org.eclipse.emf.ecore.util.InternalEList; import org.eclipse.xtext.linking.lazy.LazyLinkingResource; import com.google.common.collect.Iterables; @@ -209,43 +205,6 @@ protected final CharSequence encodeFingerprint(final ExportItem export) { } } - /** - * Return an Iterable containing all the contents of the given feature of the given object. If the - * feature is not many-valued, the resulting iterable will have one element. If the feature is an EReference, - * the iterable may contain proxies. The iterable may contain null values. - * - * @param - * The Generic type of the objects in the iterable - * @param obj - * The object - * @param feature - * The feature - * @return An iterable over all the contents of the feature of the object. - */ - @SuppressWarnings("unchecked") - private Iterable featureIterable(final EObject obj, final EStructuralFeature feature) { - if (feature == null) { - return Collections.emptyList(); - } - if (feature.isMany()) { - if (feature instanceof EAttribute || ((EReference) feature).isContainment()) { - return (Iterable) obj.eGet(feature); - } - return new Iterable() { - @Override - public Iterator iterator() { - EList list = (EList) obj.eGet(feature); - if (list instanceof InternalEList internalList) { - return internalList.basicIterator(); // Don't resolve - } else { - return list.iterator(); - } - } - }; - } - return Collections.singletonList((T) obj.eGet(feature, false)); // Don't resolve - } - /** * Generate a fingerprint for the target object using its URI. * @@ -319,7 +278,7 @@ protected CharSequence fingerprintFeature(final EObject obj, final EReference re if (obj == null) { return NULL_STRING; } - final Iterable targets = featureIterable(obj, ref); + final Iterable targets = FingerprintFeatures.featureIterable(obj, ref); if (targets == null) { return NULL_STRING; } @@ -370,7 +329,7 @@ protected CharSequence fingerprintFeature(final EObject obj, final EAttribute at if (obj == null) { return NULL_STRING; } - final Iterable values = featureIterable(obj, attr); + final Iterable values = FingerprintFeatures.featureIterable(obj, attr); if (values == null) { return NULL_STRING; } @@ -468,7 +427,7 @@ protected ExportItem fingerprintRef(final EObject obj, final EReference ref, fin if (obj == null) { return NO_EXPORT; } - final Iterable targets = featureIterable(obj, ref); + final Iterable targets = FingerprintFeatures.featureIterable(obj, ref); if (targets == null) { return NO_EXPORT; } diff --git a/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/AbstractStreamingFingerprintComputer.java b/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/AbstractStreamingFingerprintComputer.java index 24d0712e0..47594323b 100644 --- a/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/AbstractStreamingFingerprintComputer.java +++ b/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/AbstractStreamingFingerprintComputer.java @@ -11,19 +11,15 @@ package com.avaloq.tools.ddk.xtext.resource; import java.util.Collections; -import java.util.Iterator; import java.util.List; -import org.eclipse.emf.common.util.EList; import org.eclipse.emf.common.util.URI; import org.eclipse.emf.ecore.EAttribute; import org.eclipse.emf.ecore.EObject; import org.eclipse.emf.ecore.EReference; -import org.eclipse.emf.ecore.EStructuralFeature; import org.eclipse.emf.ecore.InternalEObject; import org.eclipse.emf.ecore.resource.Resource; import org.eclipse.emf.ecore.util.EcoreUtil; -import org.eclipse.emf.ecore.util.InternalEList; import org.eclipse.xtext.linking.lazy.LazyLinkingResource; import com.google.common.collect.Iterables; @@ -133,43 +129,6 @@ protected String computeFingerprint(final Iterable objects) { return hasher.hash().toString(); } - /** - * Return an Iterable containing all the contents of the given feature of the given object. If the - * feature is not many-valued, the resulting iterable will have one element. If the feature is an EReference, - * the iterable may contain proxies. The iterable may contain null values. - * - * @param - * The Generic type of the objects in the iterable - * @param obj - * The object - * @param feature - * The feature - * @return An iterable over all the contents of the feature of the object. - */ - @SuppressWarnings("unchecked") - private Iterable featureIterable(final EObject obj, final EStructuralFeature feature) { - if (feature == null) { - return Collections.emptyList(); - } - if (feature.isMany()) { - if (feature instanceof EAttribute || ((EReference) feature).isContainment()) { - return (Iterable) obj.eGet(feature); - } - return new Iterable() { - @Override - public Iterator iterator() { - EList list = (EList) obj.eGet(feature); - if (list instanceof InternalEList internalList) { - return internalList.basicIterator(); // Don't resolve - } else { - return list.iterator(); - } - } - }; - } - return Collections.singletonList((T) obj.eGet(feature, false)); // Don't resolve - } - /** * Generate a fingerprint for the target object using its URI. * @@ -249,7 +208,7 @@ protected void fingerprintFeature(final EObject obj, final EReference ref, final hasher.putUnencodedChars(NULL_STRING); return; } - final Iterable targets = featureIterable(obj, ref); + final Iterable targets = FingerprintFeatures.featureIterable(obj, ref); if (targets == null) { hasher.putUnencodedChars(NULL_STRING); return; @@ -306,7 +265,7 @@ protected void fingerprintFeature(final EObject obj, final EAttribute attr, fina hasher.putUnencodedChars(NULL_STRING); return; } - final Iterable values = featureIterable(obj, attr); + final Iterable values = FingerprintFeatures.featureIterable(obj, attr); if (values == null) { hasher.putUnencodedChars(NULL_STRING); return; @@ -409,7 +368,7 @@ protected void fingerprintRef(final EObject obj, final EReference ref, final Fin if (obj == null) { return; } - final Iterable targets = featureIterable(obj, ref); + final Iterable targets = FingerprintFeatures.featureIterable(obj, ref); if (targets == null) { return; } diff --git a/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/FingerprintFeatures.java b/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/FingerprintFeatures.java new file mode 100644 index 000000000..cb840d8df --- /dev/null +++ b/com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/FingerprintFeatures.java @@ -0,0 +1,70 @@ +/******************************************************************************* + * Copyright (c) 2016 Avaloq Group AG and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Avaloq Group AG - initial API and implementation + *******************************************************************************/ +package com.avaloq.tools.ddk.xtext.resource; + +import java.util.Collections; +import java.util.Iterator; + +import org.eclipse.emf.common.util.EList; +import org.eclipse.emf.ecore.EAttribute; +import org.eclipse.emf.ecore.EObject; +import org.eclipse.emf.ecore.EReference; +import org.eclipse.emf.ecore.EStructuralFeature; +import org.eclipse.emf.ecore.util.InternalEList; + + +/** + * Shared helpers for iterating over the contents of {@link EStructuralFeature}s when computing fingerprints. + */ +final class FingerprintFeatures { + + private FingerprintFeatures() { + // utility class + } + + /** + * Return an Iterable containing all the contents of the given feature of the given object. If the + * feature is not many-valued, the resulting iterable will have one element. If the feature is an EReference, + * the iterable may contain proxies. The iterable may contain null values. + * + * @param + * The Generic type of the objects in the iterable + * @param obj + * The object + * @param feature + * The feature + * @return An iterable over all the contents of the feature of the object. + */ + @SuppressWarnings("unchecked") + static Iterable featureIterable(final EObject obj, final EStructuralFeature feature) { + if (feature == null) { + return Collections.emptyList(); + } + if (feature.isMany()) { + if (feature instanceof EAttribute || ((EReference) feature).isContainment()) { + return (Iterable) obj.eGet(feature); + } + return new Iterable() { + @Override + public Iterator iterator() { + EList list = (EList) obj.eGet(feature); + if (list instanceof InternalEList internalList) { + return internalList.basicIterator(); // Don't resolve + } else { + return list.iterator(); + } + } + }; + } + return Collections.singletonList((T) obj.eGet(feature, false)); // Don't resolve + } + +} diff --git a/ddk-parent/pom.xml b/ddk-parent/pom.xml index 491a7ab04..09282b943 100644 --- a/ddk-parent/pom.xml +++ b/ddk-parent/pom.xml @@ -31,7 +31,7 @@ true ${workspace}/ddk-configuration/pmd/ruleset.xml - 100000 + 100 ${workspace}/ddk-configuration/checkstyle/avaloq.xml ${workspace}/ddk-configuration/checkstyle/avaloq-test.xml ${workspace}/ddk-configuration/findbugs/exclusion-filter.xml diff --git a/docs/ci-measurement-protocol.md b/docs/ci-measurement-protocol.md new file mode 100644 index 000000000..470392f48 --- /dev/null +++ b/docs/ci-measurement-protocol.md @@ -0,0 +1,65 @@ +# CI timing-measurement protocol + +How to get **trustworthy** numbers for a CI-shape change. Written because the +earlier experiment program (2026-05) produced numbers that can't be trusted: +single samples, taken during an Eclipse p2 mirror-flaky window, against a CI +shape that has since changed. Don't repeat that. + +## What we're measuring + +- **Wall-clock** per run = the slowest job (what a developer waits for). +- **Per-job duration** = the analysis bottleneck (settles e.g. spotbugs-vs-maven-verify). +- **Failure latency** = time-to-red on a *planted* lint violation — the metric the + early-fail goal actually cares about. Measure it separately. + +Wall-clock is the headline, but per-job + failure-latency are what tell you *why*. + +## Noise floor (observed on this repo's `verify` runs) + +Per-job spread across recent runs — any change must beat this to be real signal: + +| Job | median | spread (±) | +|---|---|---| +| line-endings | ~5s | ±3s | +| checkstyle (old shape) | ~115s | ±~104s (≈90%!) | +| pmd (old shape) | ~191s | ±63s | +| maven-verify | ~869s (~14.5m) | ±~93s (≈11%) | + +**Decision rule:** accept candidate B as faster than A only if +`median(B) < median(A) − 2 × IQR(A)`. For maven-verify a real win must exceed ~100s; +for checkstyle, ~200s. Anything smaller is noise. + +## Protocol + +1. **Pre-flight — confirm mirrors are healthy.** Run one throwaway build; if + target-platform resolution stalls or errors, **stop** — the window is bad + (this is what voided the 2026-05-09 numbers). Measure only on a clean window. +2. **Warm the caches.** Run 3–5 discard builds first so `~/.m2` + `.cache/tycho` + are populated; cold-cache runs have a different (network-bound) profile. +3. **Sample.** N = 15–20 `workflow_dispatch` runs per candidate, back-to-back in a + ≤30-minute window. Run A and B **interleaved within ≤10 minutes** of each other + so they see the same mirror weather and runner-pool load. +4. **Report medians + IQR**, per job *and* total wall-clock. Never a single sample, + never the mean. +5. **Pin `ubuntu-24.04`** (not a matrix) for consistent runner hardware. +6. **Record per-sample metadata:** cache hit/miss, run start time, headSha. +7. **Failure latency:** plant a synthetic PMD/Checkstyle violation, measure how long + until the `lint` check goes red (independent of the build job). + +## Why not just trust the old experiment numbers + +- Single sample each (no repetition). +- 2026-05-09 mirror-flaky window → resolution time is contaminated and varies per job + even within one dispatch. +- Numbers disagree across rounds (sequential measured 21m one round, 33m another). +- `#1369` reshaped master's CI after the experiments ran, so their baseline is stale. + +## What is *not* a timing lever (validated) + +- **Local p2 mirror**: with a warm `~/.m2`/`.cache/tycho`, offline resolution is + ~equal to online (measured 5s vs 6s) — a mirror's steady-state speedup is ~0. + Its only value is cold-cache + flaky-mirror insurance; deferred per the rare-flake + rule. +- **`-Dtycho.mode=maven` on a gate pass**: marginal on a warm cache (resolution is + already seconds); and it *breaks* PMD type-resolving rules (strips the classpath → + false positives). Not used. diff --git a/docs/ci-static-analysis-design.md b/docs/ci-static-analysis-design.md new file mode 100644 index 000000000..acbee5eec --- /dev/null +++ b/docs/ci-static-analysis-design.md @@ -0,0 +1,108 @@ +# Static-analysis CI design: SARIF inline display + count-gate + +This documents *why* the static-analysis CI is shaped the way it is, so the +mechanics (verified against the plugin source) don't have to be re-discovered. + +## Goal + +Fast, complete, **early** PMD + Checkstyle feedback — a violation should fail CI +in minutes, on its own check, not after the ~15-minute build. SpotBugs (the slow +analysis) runs in its own parallel lane so it never delays the fast checks. All +of PMD, Checkstyle, and SpotBugs surface **inline** on the PR via SARIF + GitHub +Code Scanning (no custom annotator). + +## Job shape (`verify.yml`) + +Four independent parallel jobs (no `needs:`); wall-clock = the slowest job. + +| Job | Runs | Threads | Gate | +|---|---|---|---| +| `lint` | `compile` + `pmd:pmd` + `checkstyle:checkstyle` (SARIF) + `pmd:cpd-check` | `-T 2C` | jq count of SARIF results (+ CPD `` grep) | +| `spotbugs` | `compile` + `spotbugs:spotbugs` (SARIF), `-Xmx4g` | `-T 2C` | jq count of SARIF results | +| `maven-verify` | `clean verify` (build + tests only) | none | Tycho-surefire | +| `line-endings` | `git ls-files` check | n/a | shell | + +`maven-verify` **no longer re-runs** the analysis goals (now owned by `lint` / +`spotbugs`). Analysis jobs use `-T 2C`; the test job does not (tests are not known +reliable under reactor parallelism). + +## Report goals vs. check goals (code-verified at each plugin version tag) + +| Goal | Kind | `@Execute` fork | Runs analysis? | Writes | Fails build? | SARIF-capable? | +|---|---|---|---|---|---|---| +| `pmd:pmd` | report | — | yes | `pmd.xml` (+ `pmd.sarif.json` w/ `-Dformat=…SarifRenderer`) | no | **yes** | +| `pmd:check` | check | `@Execute(goal=pmd)` | yes (forked) | `pmd.xml` | yes (`> maxAllowedViolations`, dflt 0; `failurePriority` dflt 5 = all) | no (wants `pmd.xml`) | +| `pmd:cpd` | report | — | yes | `cpd.xml` | no | no (no CPD SARIF renderer) | +| `pmd:cpd-check` | check | `@Execute(goal=cpd)` | yes (forked) | `cpd.xml` | yes | no | +| `checkstyle:checkstyle` | report | — | yes | `checkstyle-result.xml` (XML, or SARIF w/ `-Dcheckstyle.output.format=sarif`) | no | **yes** | +| `checkstyle:check` | check | **none** (self-contained) | yes (internal, source-based) | `checkstyle-result.xml` (XML) | yes (severity ≥ `violationSeverity`) | no | +| `spotbugs:spotbugs` | report | — | yes (bytecode) | `spotbugsXml.xml` (+ `spotbugsSarif.json` w/ `-Dspotbugs.sarifOutput=true`) | no | **yes** | +| `spotbugs:check` | check | `@Execute(goal=spotbugs)` | yes (forked) | `spotbugsXml.xml` | yes (`bugCount > 0`) | no | + +The split is **observe vs. enforce**: report goals produce output (for the Maven +site / dashboards / SARIF consumers) and never fail the build; check goals bind to +`verify` and exist to fail the build. The `@Execute(goal=…)` on the PMD/CPD/SpotBugs +checks forks the report goal first (so `mvn pmd:check` is self-contained) — which +means they **re-run the analysis every time**. `checkstyle:check` is the lone +exception: no `@Execute`, runs Checkstyle internally in one pass. + +## Maven reactor failure flags + +| Flag | Reactor behavior | Multi-module report completeness | Suppresses violation failure? | +|---|---|---|---| +| `--fail-fast` (default) | halt at first failing module | downstream skipped → reports missing | no | +| `--fail-at-end` | build independent modules, fail at end | modules downstream of a failure still cascade-skipped → incomplete | no | +| `--fail-never` | never fail on `MojoFailureException` (a real compile error / `MojoExecutionException` still halts) | **all** modules run → **complete** reports/SARIF | **yes** — violation failures swallowed | + +## Why `report goal + --fail-never + jq count` (the count-gate) + +We want **both** SARIF (for inline display) **and** a build gate, from **one** +analysis pass. The plugin design forces a choice: + +| Combination | Result | +|---|---| +| check goal + `--fail-fast` | gates, but first violation hides later modules' findings | +| check goal + `--fail-at-end` | gates, but cascade-skip drops downstream findings | +| check goal + `--fail-never` | doesn't gate (failure swallowed) — pointless | +| **report goal + `--fail-never` + jq count** | **all** modules analyzed → complete SARIF (full inline display); the jq count does the gating | + +So each tool runs its report goal **once** (`compile` + report, full classpath → +correct + SARIF), and a jq count over the merged SARIF fails the job. + +### Why not the `:check` goals +- They `@Execute`-fork a **second** analysis on top of the `pmd:pmd` we already run + for SARIF — pure waste. +- The forked analysis is only correct with the full `compile` + classpath; run cheaply + (`-Dtycho.mode=maven` / no compile) it loses the classpath and **false-positives** + (e.g. PMD `InvalidLogMessageFormat` flags the SLF4J trailing-`Throwable` idiom because + it can't resolve the last arg as a `Throwable`). +- `pmd:check` is **incompatible with `-Dformat=sarif`**: its fork writes `pmd.sarif.json`, + but the check looks for `pmd.xml` → "unable to find report." + +### count-gate == `:check` fidelity (for this project's config) +Counting **all** SARIF results equals what `:check` would fail on, because: PMD has no +`failurePriority` override (default 5 = all priorities), Checkstyle is globally +`severity=warning` with no info-level results, and SpotBugs uses `threshold=Low` with no +`failThreshold`/`maxAllowedViolations` override. Three config-drift caveats would make +the count-gate *stricter* than `:check` (over-fail, never under-fail), each guard-worthy: +1. PMD `pmd.failurePriority` set below 5. +2. A Checkstyle rule emitting `info`/`note` severity. +3. SpotBugs `failThreshold` or non-zero `maxAllowedViolations`. + +## Two operational rules + +- **`compile` must be full-reactor** (`-f ddk-parent/pom.xml`, no `-pl`). PMD's + type-resolving rules need the complete aux-classpath; a `-pl` subset produces + false positives (the trailing-`Throwable` case). +- **Merge SARIFs from SARIF files only.** Code Scanning accepts one run per category, + so per-module SARIFs are merged (jq) before upload. The `ddk-parent` aggregator emits + plain-XML `checkstyle-result.xml`; the merge must filter to JSON-parseable files. + +## SARIF mechanics + +- PMD: `-Dformat=net.sourceforge.pmd.renderers.SarifRenderer` → `pmd.sarif.json`. +- Checkstyle: `-Dcheckstyle.output.format=sarif` → SARIF content in `checkstyle-result.xml`. +- SpotBugs: `-Dspotbugs.sarifOutput=true` → `spotbugsSarif.json`. +- All emit SARIF 2.1.0. Merge per tool → `github/codeql-action/upload-sarif` + (`permissions: security-events: write`). CPD has no SARIF renderer → XML + grep gate + (no inline display for CPD; acceptable, duplications are rare).