Skip to content

Commit 9e73c19

Browse files
joaodinissfclaude
joaodinissf
and
claude
committed
chore: enable CPD detection (pmd.cpd.min 100000 -> 100), de-dup and baseline
pmd.cpd.min was 100000, which effectively disabled PMD's copy/paste detector (#1339). Lower it to PMD's default of 100 so CPD catches duplication going forward. At threshold 100 the reactor surfaces 9 pre-existing duplications (5 groups). Two are genuinely extractable logic and are refactored away; the other three are incidental or deliberately explicit and are baselined with // CPD-OFF markers that each carry a per-site reason. Refactored: - check.ui ExtensionHelper guard chain: the identical isExtensionUpdateRequired body in CheckValidatorExtensionHelper and CheckQuickfixExtensionHelper is extracted to a shared isTargetClassExtensionUpdateRequired method on AbstractCheckExtensionHelper (keyed on the existing getExtensionPointId() and a new protected getTargetClassName() hook); the two helpers now delegate to it. The base isExtensionUpdateRequired keeps its coarse extension-point check, which the documentation helpers rely on via super; that subtree has no target class, so it declares the getTargetClassName hook unsupported once in its shared parent (AbstractCheckDocumentationExtensionHelper). - xtext fingerprint featureIterable: the identical private helper in AbstractFingerprintComputer and AbstractStreamingFingerprintComputer is extracted to a shared package-private FingerprintFeatures utility. Baselined (CPD-OFF, with reason): - DispatchingCheckImpl: incidental token overlap (DI field + trace/try idiom), not an extractable unit. - ParameterListMatcherTest: explicit parameterized test cases, kept readable. - AbstractValidationTest: test scaffolding; per-test clarity beats extraction. Verified: check.ui and xtext compile; pmd:cpd-check is clean across the full reactor at threshold 100. Closes #1339 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e882965 commit 9e73c19

12 files changed

Lines changed: 146 additions & 115 deletions

File tree

‎com.avaloq.tools.ddk.check.runtime.core/src/com/avaloq/tools/ddk/check/runtime/issue/DispatchingCheckImpl.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
@SuppressWarnings({"checkstyle:AbstractClassName"})
3636
public abstract class DispatchingCheckImpl extends AbstractCheckImpl {
3737

38+
// CPD-OFF — incidental token overlap (DI field + trace/try idiom), not an extractable unit (#1339)
3839
@Inject
3940
private ITraceSet traceSet;
4041

@@ -72,6 +73,7 @@ public boolean validate(final EClass eClass, final EObject object, final Diagnos
7273

7374
State state = new State();
7475
state.chain = diagnostics;
76+
// CPD-ON
7577
state.eventCollector = eventCollector;
7678

7779
validate(checkMode, object, state);
@@ -123,6 +125,7 @@ protected void validate(final String contextName, final String qContextName, fin
123125
if (!disabledMethodTracker.isDisabled(contextName)) {
124126
Collector eventCollector = diagnosticCollector.getEventCollector();
125127
try {
128+
// CPD-OFF — incidental token overlap (DI field + trace/try idiom), not an extractable unit (#1339)
126129
traceStart(qContextName, object, eventCollector);
127130
checkAction.run();
128131
} catch (Exception e) {
@@ -160,6 +163,7 @@ protected static class State implements ValidationMessageAcceptorMixin, Diagnost
160163
// CHECKSTYLE:OFF
161164
public DiagnosticChain chain;
162165
public CheckType currentCheckType;
166+
// CPD-ON
163167
public boolean hasErrors;
164168
public ResourceValidationRuleSummaryEvent.Collector eventCollector;
165169
// CHECKSTYLE:ON

‎com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/AbstractCheckDocumentationExtensionHelper.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,18 @@ protected boolean isExtensionEnabled(final IPluginModelBase base, final CheckCat
4444
return !config.isGenerateLanguageInternalChecks();
4545
}
4646

47+
/**
48+
* Documentation extensions do not reference a generated target class, so documentation helpers have no target class name and
49+
* provide their own {@link #isExtensionUpdateRequired} logic that never consults it.
50+
*
51+
* @param catalog
52+
* the check catalog
53+
* @return never returns normally
54+
*/
55+
@SuppressWarnings("PMD.UnusedFormalParameter")
56+
@Override
57+
protected String getTargetClassName(final CheckCatalog catalog) {
58+
throw new UnsupportedOperationException("Documentation extension helpers have no target class"); //$NON-NLS-1$
59+
}
60+
4761
}

‎com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/AbstractCheckExtensionHelper.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,41 @@ protected boolean isExtensionUpdateRequired(final CheckCatalog catalog, final IP
147147
return extension.getPoint().equals(getExtensionPointId()); // if points are different, given extension must not be updated
148148
}
149149

150+
/**
151+
* Checks whether a class-referencing extension (validator / quickfix) needs updating: it must point at this helper's
152+
* extension point and reference exactly one element whose target class, language and catalog name still match the
153+
* check catalog. Shared by the validator and quickfix helpers, whose only difference is {@link #getTargetClassName}.
154+
*
155+
* @param catalog
156+
* the catalog
157+
* @param extension
158+
* the extension
159+
* @param elements
160+
* the elements
161+
* @return true, if the extension must be regenerated
162+
*/
163+
protected boolean isTargetClassExtensionUpdateRequired(final CheckCatalog catalog, final IPluginExtension extension, final Iterable<IPluginElement> elements) {
164+
// CHECKSTYLE:OFF
165+
// @Format-Off
166+
return getExtensionPointId().equals(extension.getPoint())
167+
&& (!extensionNameMatches(extension, catalog)
168+
|| Iterables.size(elements) != 1
169+
|| !targetClassMatches(Iterables.get(elements, 0), getTargetClassName(catalog))
170+
|| catalog.getGrammar() == null && Iterables.get(elements, 0).getAttribute(LANGUAGE_ELEMENT_TAG) != null
171+
|| catalog.getGrammar() != null && !languageNameMatches(Iterables.get(elements, 0), catalog.getGrammar().getName()));
172+
// @Format-On
173+
// CHECKSTYLE:ON
174+
}
175+
176+
/**
177+
* Gets the target class name based on the package path of given check catalog.
178+
*
179+
* @param catalog
180+
* the check catalog
181+
* @return the target class FQN
182+
*/
183+
protected abstract String getTargetClassName(CheckCatalog catalog);
184+
150185
/**
151186
* Updates a given extension to values calculated using given check catalog.
152187
*

‎com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckPreferencesExtensionHelper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,8 @@ public String getExtensionPointName(final CheckCatalog catalog) {
160160
* the check catalog
161161
* @return the target class FQN
162162
*/
163-
private String getTargetClassName(final CheckCatalog catalog) {
163+
@Override
164+
protected String getTargetClassName(final CheckCatalog catalog) {
164165
return getFromServiceProvider(CheckGeneratorNaming.class, catalog).qualifiedPreferenceInitializerClassName(catalog);
165166
}
166167

‎com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckQuickfixExtensionHelper.java‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -120,22 +120,14 @@ protected void doUpdateExtension(final CheckCatalog catalog, final IPluginExtens
120120
* the check catalog
121121
* @return the target class FQN
122122
*/
123-
private String getTargetClassName(final CheckCatalog catalog) {
123+
@Override
124+
protected String getTargetClassName(final CheckCatalog catalog) {
124125
return getFromServiceProvider(CheckGeneratorNaming.class, catalog).qualifiedQuickfixClassName(catalog);
125126
}
126127

127128
@Override
128-
public boolean isExtensionUpdateRequired(final CheckCatalog catalog, final IPluginExtension extension, final Iterable<IPluginElement> elements) {
129-
// CHECKSTYLE:OFF
130-
// @Format-Off
131-
return QUICKFIX_EXTENSION_POINT_ID.equals(extension.getPoint())
132-
&& (!extensionNameMatches(extension, catalog)
133-
|| Iterables.size(elements) != 1
134-
|| !targetClassMatches(Iterables.get(elements, 0), getTargetClassName(catalog))
135-
|| catalog.getGrammar() == null && Iterables.get(elements, 0).getAttribute(LANGUAGE_ELEMENT_TAG) != null
136-
|| catalog.getGrammar() != null && !languageNameMatches(Iterables.get(elements, 0), catalog.getGrammar().getName()));
137-
// @Format-On
138-
// CHECKSTYLE:ON
129+
protected boolean isExtensionUpdateRequired(final CheckCatalog catalog, final IPluginExtension extension, final Iterable<IPluginElement> elements) {
130+
return isTargetClassExtensionUpdateRequired(catalog, extension, elements);
139131
}
140132

141133
}

‎com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/builder/util/CheckValidatorExtensionHelper.java‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -112,23 +112,14 @@ private String getCatalogResourceName(final CheckCatalog catalog) {
112112
* the check catalog
113113
* @return the target class FQN
114114
*/
115-
private String getTargetClassName(final CheckCatalog catalog) {
115+
@Override
116+
protected String getTargetClassName(final CheckCatalog catalog) {
116117
return getFromServiceProvider(CheckGeneratorNaming.class, catalog).qualifiedValidatorClassName(catalog);
117118
}
118119

119120
@Override
120-
public boolean isExtensionUpdateRequired(final CheckCatalog catalog, final IPluginExtension extension, final Iterable<IPluginElement> elements) {
121-
// CHECKSTYLE:OFF
122-
// @Format-Off
123-
return CHECK_EXTENSION_POINT_ID.equals(extension.getPoint())
124-
&& (!extensionNameMatches(extension, catalog)
125-
|| Iterables.size(elements) != 1
126-
|| !targetClassMatches(Iterables.get(elements, 0), getTargetClassName(catalog))
127-
|| catalog.getGrammar() == null && Iterables.get(elements, 0).getAttribute(LANGUAGE_ELEMENT_TAG) != null
128-
|| catalog.getGrammar() != null && !languageNameMatches(Iterables.get(elements, 0), catalog.getGrammar().getName())
129-
);
130-
// @Format-On
131-
// CHECKSTYLE:ON
121+
protected boolean isExtensionUpdateRequired(final CheckCatalog catalog, final IPluginExtension extension, final Iterable<IPluginElement> elements) {
122+
return isTargetClassExtensionUpdateRequired(catalog, extension, elements);
132123
}
133124

134125
}

‎com.avaloq.tools.ddk.typesystem.test/src/com/avaloq/tools/ddk/typesystem/ParameterListMatcherTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,7 @@ void testUnnamedFormalAfterNamed4() {
893893
assertSame(unnamedFormal2, matchResult.getUnnamedFormalsAfterNamed().get(1), UNNAMED_FORMAL_AFTER_NAMED_NOT_LOCATED);
894894
}
895895

896+
// CPD-OFF — explicit parameterized test cases, kept readable over shared (#1339)
896897
@Test
897898
void testForceMatchByPosition1() {
898899
List<NamedFormalParameter> formals = new ArrayList<NamedFormalParameter>();
@@ -946,5 +947,6 @@ void testForceMatchByPosition3() {
946947
checkParameterMatch(IParameterMatchChecker.MatchStatus.MATCH, actuals.get(1), formals.get(1), matches.get(1));
947948
checkParameterMatch(IParameterMatchChecker.MatchStatus.MATCH, actuals.get(2), formals.get(2), matches.get(2));
948949
}
950+
// CPD-ON
949951

950952
}

‎com.avaloq.tools.ddk.xtext.test.core/src/com/avaloq/tools/ddk/xtext/test/jupiter/AbstractValidationTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,7 @@ public void apply(final EObject root, final Integer pos) {
377377
* actual message
378378
*/
379379
private void createErrorMessage(final Integer pos, final List<Resource.Diagnostic> diagnosticsOnTargetPosition, final boolean issueFound, final boolean expectedSeverityMatches, final int actualSeverity, final boolean expectedMessageMatches, final String actualMessage) {
380+
// CPD-OFF — test scaffolding; per-test clarity beats extraction (#1339)
380381
StringBuilder errorMessage = new StringBuilder(200);
381382
if (issueMustBeFound && !issueFound) {
382383
errorMessage.append("Expected issue not found. Code '").append(issueCode).append('\n');
@@ -398,6 +399,7 @@ private void createErrorMessage(final Integer pos, final List<Resource.Diagnosti
398399
}
399400
memorizeErrorOnPosition(pos, errorMessage.toString());
400401
}
402+
// CPD-ON
401403
}
402404

403405
/**
@@ -1002,6 +1004,7 @@ public static void assertNoLinkingErrorsOnResource(final EObject object, final S
10021004
*/
10031005
@SuppressWarnings("PMD.UnusedFormalParameter")
10041006
public static void assertLinkingErrorsOnResourceExist(final EObject object, final String referenceType, final String... referenceNames) {
1007+
// CPD-OFF — test scaffolding; per-test clarity beats extraction (#1339)
10051008
final List<Resource.Diagnostic> linkingErrors = object.eResource().getErrors().stream().filter(error -> error instanceof XtextLinkingDiagnostic).collect(Collectors.toList());
10061009
final List<String> errorMessages = Lists.transform(linkingErrors, Resource.Diagnostic::getMessage);
10071010
for (final String referenceName : referenceNames) {
@@ -1012,6 +1015,7 @@ public static void assertLinkingErrorsOnResourceExist(final EObject object, fina
10121015
break;
10131016
}
10141017
}
1018+
// CPD-ON
10151019
assertTrue(found, NLS.bind("Expected linking error on \"{0}\" but could not find it", referenceName));
10161020
}
10171021
}

‎com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/AbstractFingerprintComputer.java‎

Lines changed: 3 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,15 @@
1414
import java.security.MessageDigest;
1515
import java.security.NoSuchAlgorithmException;
1616
import java.util.Collections;
17-
import java.util.Iterator;
1817
import java.util.List;
1918

20-
import org.eclipse.emf.common.util.EList;
2119
import org.eclipse.emf.common.util.URI;
2220
import org.eclipse.emf.ecore.EAttribute;
2321
import org.eclipse.emf.ecore.EObject;
2422
import org.eclipse.emf.ecore.EReference;
25-
import org.eclipse.emf.ecore.EStructuralFeature;
2623
import org.eclipse.emf.ecore.InternalEObject;
2724
import org.eclipse.emf.ecore.resource.Resource;
2825
import org.eclipse.emf.ecore.util.EcoreUtil;
29-
import org.eclipse.emf.ecore.util.InternalEList;
3026
import org.eclipse.xtext.linking.lazy.LazyLinkingResource;
3127

3228
import com.google.common.collect.Iterables;
@@ -209,43 +205,6 @@ protected final CharSequence encodeFingerprint(final ExportItem export) {
209205
}
210206
}
211207

212-
/**
213-
* Return an Iterable containing all the contents of the given feature of the given object. If the
214-
* feature is not many-valued, the resulting iterable will have one element. If the feature is an EReference,
215-
* the iterable may contain proxies. The iterable may contain null values.
216-
*
217-
* @param <T>
218-
* The Generic type of the objects in the iterable
219-
* @param obj
220-
* The object
221-
* @param feature
222-
* The feature
223-
* @return An iterable over all the contents of the feature of the object.
224-
*/
225-
@SuppressWarnings("unchecked")
226-
private <T> Iterable<T> featureIterable(final EObject obj, final EStructuralFeature feature) {
227-
if (feature == null) {
228-
return Collections.emptyList();
229-
}
230-
if (feature.isMany()) {
231-
if (feature instanceof EAttribute || ((EReference) feature).isContainment()) {
232-
return (Iterable<T>) obj.eGet(feature);
233-
}
234-
return new Iterable<T>() {
235-
@Override
236-
public Iterator<T> iterator() {
237-
EList<T> list = (EList<T>) obj.eGet(feature);
238-
if (list instanceof InternalEList<T> internalList) {
239-
return internalList.basicIterator(); // Don't resolve
240-
} else {
241-
return list.iterator();
242-
}
243-
}
244-
};
245-
}
246-
return Collections.singletonList((T) obj.eGet(feature, false)); // Don't resolve
247-
}
248-
249208
/**
250209
* Generate a fingerprint for the target object using its URI.
251210
*
@@ -319,7 +278,7 @@ protected CharSequence fingerprintFeature(final EObject obj, final EReference re
319278
if (obj == null) {
320279
return NULL_STRING;
321280
}
322-
final Iterable<? extends EObject> targets = featureIterable(obj, ref);
281+
final Iterable<? extends EObject> targets = FingerprintFeatures.featureIterable(obj, ref);
323282
if (targets == null) {
324283
return NULL_STRING;
325284
}
@@ -370,7 +329,7 @@ protected CharSequence fingerprintFeature(final EObject obj, final EAttribute at
370329
if (obj == null) {
371330
return NULL_STRING;
372331
}
373-
final Iterable<? extends Object> values = featureIterable(obj, attr);
332+
final Iterable<? extends Object> values = FingerprintFeatures.featureIterable(obj, attr);
374333
if (values == null) {
375334
return NULL_STRING;
376335
}
@@ -468,7 +427,7 @@ protected ExportItem fingerprintRef(final EObject obj, final EReference ref, fin
468427
if (obj == null) {
469428
return NO_EXPORT;
470429
}
471-
final Iterable<? extends EObject> targets = featureIterable(obj, ref);
430+
final Iterable<? extends EObject> targets = FingerprintFeatures.featureIterable(obj, ref);
472431
if (targets == null) {
473432
return NO_EXPORT;
474433
}

‎com.avaloq.tools.ddk.xtext/src/com/avaloq/tools/ddk/xtext/resource/AbstractStreamingFingerprintComputer.java‎

Lines changed: 3 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,15 @@
1111
package com.avaloq.tools.ddk.xtext.resource;
1212

1313
import java.util.Collections;
14-
import java.util.Iterator;
1514
import java.util.List;
1615

17-
import org.eclipse.emf.common.util.EList;
1816
import org.eclipse.emf.common.util.URI;
1917
import org.eclipse.emf.ecore.EAttribute;
2018
import org.eclipse.emf.ecore.EObject;
2119
import org.eclipse.emf.ecore.EReference;
22-
import org.eclipse.emf.ecore.EStructuralFeature;
2320
import org.eclipse.emf.ecore.InternalEObject;
2421
import org.eclipse.emf.ecore.resource.Resource;
2522
import org.eclipse.emf.ecore.util.EcoreUtil;
26-
import org.eclipse.emf.ecore.util.InternalEList;
2723
import org.eclipse.xtext.linking.lazy.LazyLinkingResource;
2824

2925
import com.google.common.collect.Iterables;
@@ -133,43 +129,6 @@ protected String computeFingerprint(final Iterable<? extends EObject> objects) {
133129
return hasher.hash().toString();
134130
}
135131

136-
/**
137-
* Return an Iterable containing all the contents of the given feature of the given object. If the
138-
* feature is not many-valued, the resulting iterable will have one element. If the feature is an EReference,
139-
* the iterable may contain proxies. The iterable may contain null values.
140-
*
141-
* @param <T>
142-
* The Generic type of the objects in the iterable
143-
* @param obj
144-
* The object
145-
* @param feature
146-
* The feature
147-
* @return An iterable over all the contents of the feature of the object.
148-
*/
149-
@SuppressWarnings("unchecked")
150-
private <T> Iterable<T> featureIterable(final EObject obj, final EStructuralFeature feature) {
151-
if (feature == null) {
152-
return Collections.emptyList();
153-
}
154-
if (feature.isMany()) {
155-
if (feature instanceof EAttribute || ((EReference) feature).isContainment()) {
156-
return (Iterable<T>) obj.eGet(feature);
157-
}
158-
return new Iterable<T>() {
159-
@Override
160-
public Iterator<T> iterator() {
161-
EList<T> list = (EList<T>) obj.eGet(feature);
162-
if (list instanceof InternalEList<T> internalList) {
163-
return internalList.basicIterator(); // Don't resolve
164-
} else {
165-
return list.iterator();
166-
}
167-
}
168-
};
169-
}
170-
return Collections.singletonList((T) obj.eGet(feature, false)); // Don't resolve
171-
}
172-
173132
/**
174133
* Generate a fingerprint for the target object using its URI.
175134
*
@@ -249,7 +208,7 @@ protected void fingerprintFeature(final EObject obj, final EReference ref, final
249208
hasher.putUnencodedChars(NULL_STRING);
250209
return;
251210
}
252-
final Iterable<? extends EObject> targets = featureIterable(obj, ref);
211+
final Iterable<? extends EObject> targets = FingerprintFeatures.featureIterable(obj, ref);
253212
if (targets == null) {
254213
hasher.putUnencodedChars(NULL_STRING);
255214
return;
@@ -306,7 +265,7 @@ protected void fingerprintFeature(final EObject obj, final EAttribute attr, fina
306265
hasher.putUnencodedChars(NULL_STRING);
307266
return;
308267
}
309-
final Iterable<? extends Object> values = featureIterable(obj, attr);
268+
final Iterable<? extends Object> values = FingerprintFeatures.featureIterable(obj, attr);
310269
if (values == null) {
311270
hasher.putUnencodedChars(NULL_STRING);
312271
return;
@@ -409,7 +368,7 @@ protected void fingerprintRef(final EObject obj, final EReference ref, final Fin
409368
if (obj == null) {
410369
return;
411370
}
412-
final Iterable<? extends EObject> targets = featureIterable(obj, ref);
371+
final Iterable<? extends EObject> targets = FingerprintFeatures.featureIterable(obj, ref);
413372
if (targets == null) {
414373
return;
415374
}

0 commit comments

Comments
 (0)