Fixed: [ Issue 60 ] Imported types do not need to be fully-qualified
https://code.google.com/p/protobuf-dt/issues/detail?id=60
Package resolution is done correctly now.
diff --git a/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/scoping/LocalNamesProvider_localNames_Test.java b/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/scoping/LocalNamesProvider_namesOf_Test.java
similarity index 92%
rename from com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/scoping/LocalNamesProvider_localNames_Test.java
rename to com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/scoping/LocalNamesProvider_namesOf_Test.java
index 2a32898..55e1202 100644
--- a/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/scoping/LocalNamesProvider_localNames_Test.java
+++ b/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/scoping/LocalNamesProvider_namesOf_Test.java
@@ -23,11 +23,11 @@
import com.google.eclipse.protobuf.protobuf.Enum;
/**
- * Tests for <code>{@link LocalNamesProvider#localNames(EObject)}</code>.
+ * Tests for <code>{@link LocalNamesProvider#namesOf(EObject)}</code>.
*
* @author alruiz@google.com (Alex Ruiz)
*/
-public class LocalNamesProvider_localNames_Test {
+public class LocalNamesProvider_namesOf_Test {
@Rule public XtextRule xtext = new XtextRule();
@@ -53,7 +53,7 @@
proto.append("} ");
Protobuf root = xtext.parse(proto);
Enum phoneType = findEnum("PhoneType", root);
- List<QualifiedName> names = namesProvider.localNames(phoneType);
+ List<QualifiedName> names = namesProvider.namesOf(phoneType);
assertThat(names.get(0).toString(), equalTo("PhoneType"));
assertThat(names.get(1).toString(), equalTo("PhoneNumber.PhoneType"));
assertThat(names.get(2).toString(), equalTo("Person.PhoneNumber.PhoneType"));
diff --git a/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/scoping/PackageResolver_areRelated_Test.java b/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/scoping/PackageResolver_areRelated_Test.java
index 23a7935..284a1cf 100644
--- a/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/scoping/PackageResolver_areRelated_Test.java
+++ b/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/scoping/PackageResolver_areRelated_Test.java
@@ -29,11 +29,21 @@
resolver = new PackageResolver();
}
+ private String baseName;
+ private String[] subpackageNames;
private Package p1;
private Package p2;
@Before public void setUp() {
- p1 = new PackageStub("may.the.force.be.with.you");
+ baseName = "may.the.force.be.with.you";
+ subpackageNames = new String[] {
+ "may.the.force.be.with",
+ "may.the.force.be.",
+ "may.the.force.",
+ "may.the",
+ "may"
+ };
+ p1 = new PackageStub(baseName);
p2 = new PackageStub();
}
@@ -41,4 +51,29 @@
p2.setName(p1.getName());
assertThat(resolver.areRelated(p1, p2), equalTo(true));
}
+
+ @Test public void should_return_true_second_is_subPackage_of_first() {
+ for (String name : subpackageNames) {
+ p2.setName(name);
+ assertThat(resolver.areRelated(p1, p2), equalTo(true));
+ }
+ }
+
+ @Test public void should_return_true_first_is_subPackage_of_second() {
+ p2.setName(baseName);
+ for (String name : subpackageNames) {
+ p1.setName(name);
+ assertThat(resolver.areRelated(p1, p2), equalTo(true));
+ }
+ }
+
+ @Test public void should_return_false_if_second_starts_with_few_segments_of_first_but_is_not_subpackage() {
+ p2.setName("may.the.ring");
+ assertThat(resolver.areRelated(p1, p2), equalTo(false));
+ }
+
+ @Test public void should_return_false_if_names_are_completely_different() {
+ p2.setName("peace.dog");
+ assertThat(resolver.areRelated(p1, p2), equalTo(false));
+ }
}
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/LocalNamesProvider.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/LocalNamesProvider.java
index 5d4f98a..98ccb0b 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/LocalNamesProvider.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/LocalNamesProvider.java
@@ -57,50 +57,53 @@
*/
class LocalNamesProvider {
- @Inject private final IQualifiedNameConverter converter = new IQualifiedNameConverter.DefaultImpl();
@Inject private final IResourceScopeCache cache = IResourceScopeCache.NullImpl.INSTANCE;
+ @Inject private final IQualifiedNameConverter converter = new IQualifiedNameConverter.DefaultImpl();
@Inject private ProtobufElementFinder finder;
private final Function<EObject, String> resolver = newResolver(String.class, "name");
- List<QualifiedName> localNames(final EObject obj) {
+ List<QualifiedName> namesOf(final EObject obj) {
Pair<EObject, String> key = pair(obj, "fqns");
return cache.get(key, obj.eResource(), new Provider<List<QualifiedName>>() {
public List<QualifiedName> get() {
- List<QualifiedName> names = new ArrayList<QualifiedName>();
+ List<QualifiedName> allNames = new ArrayList<QualifiedName>();
EObject current = obj;
String name = resolver.apply(current);
- if (isEmpty(name)) return names;
+ if (isEmpty(name)) return emptyList();
QualifiedName qualifiedName = converter.toQualifiedName(name);
- names.add(qualifiedName);
+ allNames.add(qualifiedName);
while (current.eContainer() != null) {
current = current.eContainer();
String containerName = resolver.apply(current);
if (isEmpty(containerName)) continue;
qualifiedName = converter.toQualifiedName(containerName).append(qualifiedName);
- names.add(qualifiedName);
+ allNames.add(qualifiedName);
}
- names.addAll(addPackageSegments(qualifiedName));
- return unmodifiableList(names);
- }
-
- private List<QualifiedName> addPackageSegments(QualifiedName qualifiedName) {
- Package p = finder.packageOf(obj);
- if (p == null) return emptyList();
- String packageName = p.getName();
- if (isEmpty(packageName)) return emptyList();
- QualifiedName name = qualifiedName;
- List<String> segments = converter.toQualifiedName(packageName).getSegments();
- int segmentCount = segments.size();
- if (segmentCount == 1) return emptyList();
- List<QualifiedName> names = new ArrayList<QualifiedName>();
- for (int i = segmentCount - 1; i > 0; i--) {
- name = QualifiedName.create(segments.get(i)).append(name);
- names.add(name);
- }
- return unmodifiableList(names);
+ allNames.addAll(addPackageNameSegments(qualifiedName, finder.packageOf(obj)));
+ return unmodifiableList(allNames);
}
});
}
+
+ private List<QualifiedName> addPackageNameSegments(QualifiedName qualifiedName, Package p) {
+ QualifiedName name = qualifiedName;
+ List<String> segments = fqnSegments(p);
+ int segmentCount = segments.size();
+ if (segmentCount <= 1) return emptyList();
+ List<QualifiedName> allNames = new ArrayList<QualifiedName>();
+ for (int i = segmentCount - 1; i > 0; i--) {
+ name = QualifiedName.create(segments.get(i)).append(name);
+ allNames.add(name);
+ }
+ return unmodifiableList(allNames);
+ }
+
+ private List<String> fqnSegments(Package p) {
+ if (p == null) return emptyList();
+ String packageName = p.getName();
+ if (isEmpty(packageName)) return emptyList();
+ return converter.toQualifiedName(packageName).getSegments();
+ }
}
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/PackageResolver.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/PackageResolver.java
index 16b21b8..1044fd6 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/PackageResolver.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/PackageResolver.java
@@ -10,15 +10,16 @@
import static org.eclipse.xtext.util.Strings.isEmpty;
+import java.util.List;
+
import org.eclipse.xtext.naming.*;
import com.google.eclipse.protobuf.protobuf.Package;
-import com.google.inject.*;
+import com.google.inject.Inject;
/**
* @author alruiz@google.com (Alex Ruiz)
*/
-@Singleton
class PackageResolver {
@Inject private final IQualifiedNameConverter converter = new IQualifiedNameConverter.DefaultImpl();
@@ -29,7 +30,7 @@
QualifiedName name2 = nameOf(p2);
if (name1 == null || name2 == null) return false;
if (name1.equals(name2)) return true;
- return false;
+ return (isSubPackage(name1, name2));
}
private QualifiedName nameOf(Package p) {
@@ -37,4 +38,15 @@
if (isEmpty(name)) return null;
return converter.toQualifiedName(name);
}
+
+ private boolean isSubPackage(QualifiedName name1, QualifiedName name2) {
+ List<String> segments2 = name2.getSegments();
+ int segment2Count = segments2.size();
+ int counter = 0;
+ for (String segment1 : name1.getSegments()) {
+ if (!segment1.equals(segments2.get(counter++))) return false;
+ if (counter == segment2Count) break;
+ }
+ return true;
+ }
}
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/ProtobufScopeProvider.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/ProtobufScopeProvider.java
index 1f9b150..9758fe2 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/ProtobufScopeProvider.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/scoping/ProtobufScopeProvider.java
@@ -8,6 +8,7 @@
*/
package com.google.eclipse.protobuf.scoping;
+import static java.util.Collections.emptyList;
import static org.eclipse.emf.common.util.URI.createURI;
import static org.eclipse.emf.ecore.util.EcoreUtil.getAllContents;
import static org.eclipse.xtext.resource.EObjectDescription.create;
@@ -16,14 +17,15 @@
import org.eclipse.emf.common.util.*;
import org.eclipse.emf.ecore.*;
-import org.eclipse.emf.ecore.resource.*;
+import org.eclipse.emf.ecore.resource.ResourceSet;
import org.eclipse.xtext.naming.*;
-import org.eclipse.xtext.resource.IEObjectDescription;
+import org.eclipse.xtext.resource.*;
import org.eclipse.xtext.scoping.IScope;
import org.eclipse.xtext.scoping.impl.*;
import com.google.eclipse.protobuf.protobuf.*;
import com.google.eclipse.protobuf.protobuf.Enum;
+import com.google.eclipse.protobuf.protobuf.Package;
import com.google.eclipse.protobuf.util.ProtobufElementFinder;
import com.google.inject.Inject;
@@ -43,6 +45,7 @@
@Inject private IQualifiedNameProvider nameProvider;
@Inject private ImportUriResolver uriResolver;
@Inject private LocalNamesProvider localNamesProvider;
+ @Inject private PackageResolver packageResolver;
@SuppressWarnings("unused")
IScope scope_TypeReference_type(TypeReference typeRef, EReference reference) {
@@ -82,7 +85,7 @@
List<IEObjectDescription> descriptions = new ArrayList<IEObjectDescription>();
for (EObject element : root.eContents()) {
if (!targetType.isInstance(element)) continue;
- List<QualifiedName> names = localNamesProvider.localNames(element);
+ List<QualifiedName> names = localNamesProvider.namesOf(element);
int nameCount = names.size();
for (int i = level; i < nameCount; i++) {
descriptions.add(create(names.get(i), element));
@@ -97,17 +100,34 @@
}
private <T extends Type> Collection<IEObjectDescription> importedTypes(Protobuf root, Class<T> targetType) {
- ResourceSet resourceSet = root.eResource().getResourceSet();
+ List<Import> imports = finder.importsIn(root);
+ if (imports.isEmpty()) return emptyList();
+ Package importRootPackage = finder.packageOf(root);
List<IEObjectDescription> descriptions = new ArrayList<IEObjectDescription>();
- for (Import anImport : finder.importsIn(root)) {
- URI importUri = createURI(uriResolver.apply(anImport));
- Resource imported = resourceSet.getResource(importUri, true);
- descriptions.addAll(innerTypes(imported, targetType));
+ for (Import anImport : imports) {
+ XtextResource imported = importedResource(anImport);
+ EObject importedRoot = imported.getParseResult().getRootASTElement();
+ if (arePackagesRelated(importRootPackage, importedRoot)) {
+ descriptions.addAll(typesIn(importedRoot));
+ continue;
+ }
+ descriptions.addAll(children(imported, targetType));
}
return descriptions;
}
- private <T extends Type> Collection<IEObjectDescription> innerTypes(Resource resource, Class<T> targetType) {
+ private XtextResource importedResource(Import anImport) {
+ ResourceSet resourceSet = finder.rootOf(anImport).eResource().getResourceSet();
+ URI importUri = createURI(uriResolver.apply(anImport));
+ return (XtextResource) resourceSet.getResource(importUri, true);
+ }
+
+ private boolean arePackagesRelated(Package aPackage, EObject root) {
+ Package p = finder.packageOf(root);
+ return packageResolver.areRelated(aPackage, p);
+ }
+
+ private <T extends Type> Collection<IEObjectDescription> children(XtextResource resource, Class<T> targetType) {
List<IEObjectDescription> descriptions = new ArrayList<IEObjectDescription>();
TreeIterator<Object> contents = getAllContents(resource, true);
while (contents.hasNext()) {
