Skip to content

Commit

Permalink
@classid duplicate finder does not find all duplicates
Browse files Browse the repository at this point in the history
The duplicate finder misses some results because of wrong classpath
calculations in IJavaProject#isOnClasspath. Try to workaround for
workspace resources using the surrounding project instead.
  • Loading branch information
mvilliger committed Sep 29, 2017
1 parent 813b600 commit 373baee
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@
import org.eclipse.jdt.core.IElementChangedListener;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.IJavaElementDelta;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.ISourceRange;
import org.eclipse.jdt.core.IType;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.Signature;
import org.eclipse.jdt.core.SourceRange;
import org.eclipse.jdt.core.search.IJavaSearchConstants;
import org.eclipse.jdt.core.search.IJavaSearchScope;
import org.eclipse.jdt.core.search.SearchEngine;
Expand All @@ -47,12 +49,13 @@
import org.eclipse.jdt.core.search.TypeReferenceMatch;
import org.eclipse.jface.text.BadLocationException;
import org.eclipse.jface.text.Document;
import org.eclipse.jface.text.IDocument;
import org.eclipse.scout.sdk.core.s.IScoutRuntimeTypes;
import org.eclipse.scout.sdk.core.util.SdkException;
import org.eclipse.scout.sdk.core.util.SdkLog;
import org.eclipse.scout.sdk.s2e.job.AbstractJob;
import org.eclipse.scout.sdk.s2e.job.ResourceBlockingOperationJob;
import org.eclipse.scout.sdk.s2e.util.S2eUtils;
import org.eclipse.scout.sdk.s2e.util.ScoutStatus;

/**
* <h3>{@link ClassIdValidationJob}</h3>
Expand Down Expand Up @@ -185,36 +188,41 @@ public static synchronized void uninstall() {
return ids;
}

private static boolean hasVisibleClassIds(IAnnotation current, List<IAnnotation> matchesById) {
for (IAnnotation m : matchesById) {
if (m != current && S2eUtils.isOnClasspath(m, current.getJavaProject())) {
return true;
private static IAnnotation getVisibleDuplicate(final IJavaElement current, final Iterable<IAnnotation> matchesById) {
final IJavaProject jp = current.getJavaProject();
for (final IAnnotation m : matchesById) {
if (m != current && S2eUtils.isOnClasspath(m, jp)) {
return m;
}
}
return false;
return null;
}

private static void createDuplicateMarkers(Map<String, List<IAnnotation>> annotations) throws CoreException {
for (Entry<String, List<IAnnotation>> matches : annotations.entrySet()) {
List<IAnnotation> matchesById = matches.getValue();
if (matchesById.size() > 1) {
for (IAnnotation duplicate : matchesById) {
IType parent = (IType) duplicate.getAncestor(IJavaElement.TYPE);
if (S2eUtils.exists(parent) && hasVisibleClassIds(duplicate, matchesById)) {
ISourceRange sourceRange = duplicate.getSourceRange();
if (sourceRange != null && sourceRange.getOffset() >= 0) {
IMarker marker = duplicate.getResource().createMarker(CLASS_ID_DUPLICATE_MARKER_ID);
marker.setAttribute(IMarker.MESSAGE, "Duplicate @ClassId value '" + matches.getKey() + "' in type '" + parent.getFullyQualifiedName() + "'.");
for (final IAnnotation duplicate : matchesById) {
final IAnnotation other = getVisibleDuplicate(duplicate, matchesById);
final IType parent = (IType) duplicate.getAncestor(IJavaElement.TYPE);
if (S2eUtils.exists(parent) && S2eUtils.exists(other)) {
@SuppressWarnings("squid:S2259")
final IType otherParent = (IType) other.getAncestor(IJavaElement.TYPE);
final ISourceRange sourceRange = duplicate.getSourceRange();
if (S2eUtils.exists(otherParent) && SourceRange.isAvailable(sourceRange)) {
final IMarker marker = duplicate.getResource().createMarker(CLASS_ID_DUPLICATE_MARKER_ID);
marker.setAttribute(IMarker.MESSAGE, "Duplicate @ClassId. Value '" + matches.getKey() + "' of type '" + parent.getFullyQualifiedName()
+ "' is the same as of type '" + otherParent.getFullyQualifiedName() + "'.");
marker.setAttribute(IMarker.PRIORITY, IMarker.PRIORITY_HIGH);
marker.setAttribute(IMarker.CHAR_START, sourceRange.getOffset());
marker.setAttribute(IMarker.CHAR_END, sourceRange.getOffset() + sourceRange.getLength());
marker.setAttribute(IMarker.SEVERITY, IMarker.SEVERITY_ERROR);
try {
Document doc = new Document(parent.getCompilationUnit().getSource());
final IDocument doc = new Document(parent.getCompilationUnit().getSource());
marker.setAttribute(IMarker.LINE_NUMBER, doc.getLineOfOffset(sourceRange.getOffset()) + 1);
}
catch (BadLocationException e) {
throw new CoreException(new ScoutStatus(e));
catch (final BadLocationException e) {
throw new SdkException(e);
}
marker.setAttribute(CLASS_ID_ATTR_ANNOTATION, duplicate);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,12 @@ public static boolean isOnClasspath(IJavaElement element, IJavaProject project)
if (!exists(element) || !exists(project)) {
return false;
}
final boolean isInWorkspace = element.getResource() != null;
if (isInWorkspace) {
// if the element is in the workspace (not binary in a library): check on the project level. Otherwise the results of IJavaProject#isOnClasspath() may be wrong!
// do not calculate the classpath visibility based on the project for binary types because their project may be any project having this dependency which may be wrong.
return project.isOnClasspath(element.getJavaProject());
}
return project.isOnClasspath(element);
}

Expand Down

0 comments on commit 373baee

Please sign in to comment.