From 320241d107fa916242f9b43da7e22722924fe1b2 Mon Sep 17 00:00:00 2001 From: Andrey Breslav Date: Thu, 29 Mar 2012 16:18:42 +0400 Subject: [PATCH] Proper error reporting when a package name redeclares something, wrt the package name occurring in multiple files See MultiFilePackageRedeclaration.jet test --- .../RedeclarationDiagnosticFactory.java | 12 ---- .../jet/lang/resolve/BindingContext.java | 3 +- .../jet/lang/resolve/DeclarationResolver.java | 57 +++++++++++++++++++ .../lang/resolve/NamespaceFactoryImpl.java | 19 +++++-- .../TraceBasedRedeclarationHandler.java | 8 +-- .../lang/resolve/scopes/WritableScope.java | 4 ++ .../resolve/scopes/WritableScopeImpl.java | 21 +++++-- .../resolve/scopes/WriteThroughScope.java | 7 +++ .../ControlStructureTypingVisitor.java | 3 +- .../ExpressionTypingVisitorForStatements.java | 5 +- .../MultiFilePackageRedeclaration.jet | 7 +++ .../{ => redeclarations}/Redeclarations.jet | 4 +- 12 files changed, 120 insertions(+), 30 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/redeclarations/MultiFilePackageRedeclaration.jet rename compiler/testData/diagnostics/tests/{ => redeclarations}/Redeclarations.jet (61%) diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/RedeclarationDiagnosticFactory.java b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/RedeclarationDiagnosticFactory.java index 7961088eb91..053238f2839 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/RedeclarationDiagnosticFactory.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/RedeclarationDiagnosticFactory.java @@ -16,16 +16,8 @@ package org.jetbrains.jet.lang.diagnostics; -import com.intellij.openapi.util.TextRange; import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; -import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; -import org.jetbrains.jet.lang.psi.JetFile; -import org.jetbrains.jet.lang.psi.JetNamedDeclaration; -import org.jetbrains.jet.lang.resolve.BindingContext; - -import java.util.Collections; -import java.util.List; /** * @author abreslav @@ -51,10 +43,6 @@ public class RedeclarationDiagnosticFactory extends AbstractDiagnosticFactory { return new RedeclarationDiagnostic.SimpleRedeclarationDiagnostic(duplicatingElement, name, this); } - public Diagnostic on(DeclarationDescriptor duplicatingDescriptor, BindingContext contextToResolveToDeclaration) { - return new RedeclarationDiagnostic.RedeclarationDiagnosticWithDeferredResolution(duplicatingDescriptor, contextToResolveToDeclaration, this); - } - @NotNull @Override public String getName() { diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java index cf6ba13c1e1..020860f9867 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java @@ -144,7 +144,6 @@ public interface BindingContext { return declarationDescriptor.getOriginal(); } }; - WritableSlice FILE_TO_NAMESPACE = Slices.createSimpleSlice(); ReadOnlySlice DESCRIPTOR_TO_DECLARATION = Slices.sliceBuilder().setKeyNormalizer(DECLARATION_DESCRIPTOR_NORMALIZER).build(); @@ -173,6 +172,8 @@ public interface BindingContext { WritableSlice FQNAME_TO_CLASS_DESCRIPTOR = new BasicWritableSlice(DO_NOTHING, true); WritableSlice FQNAME_TO_NAMESPACE_DESCRIPTOR = new BasicWritableSlice(DO_NOTHING); + WritableSlice FILE_TO_NAMESPACE = Slices.createSimpleSlice(); + WritableSlice> NAMESPACE_TO_FILES = Slices.createSimpleSlice(); /** * Each namespace found in src must be registered here. diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java index 04de9ed92b0..2c04a5c459e 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java @@ -16,17 +16,26 @@ package org.jetbrains.jet.lang.resolve; +import com.google.common.base.Function; +import com.google.common.base.Predicate; +import com.google.common.collect.Collections2; +import com.google.common.collect.Multimap; +import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.*; import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.resolve.scopes.JetScope; import org.jetbrains.jet.lang.resolve.scopes.WritableScope; import javax.inject.Inject; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import static org.jetbrains.jet.lang.diagnostics.Errors.CONSTRUCTOR_IN_TRAIT; +import static org.jetbrains.jet.lang.diagnostics.Errors.REDECLARATION; import static org.jetbrains.jet.lang.diagnostics.Errors.SECONDARY_CONSTRUCTORS_ARE_NOT_SUPPORTED; /** @@ -77,8 +86,11 @@ public class DeclarationResolver { resolveAnnotationStubsOnClassesAndConstructors(); resolveFunctionAndPropertyHeaders(); importsResolver.processMembersImports(); + checkRedeclarationsInNamespaces(); } + + private void resolveConstructorHeaders() { for (Map.Entry entry : context.getClasses().entrySet()) { JetClass jetClass = entry.getKey(); @@ -233,4 +245,49 @@ public class DeclarationResolver { context.getDeclaringScopes().put(constructor, classDescriptor.getScopeForMemberLookup()); } + private void checkRedeclarationsInNamespaces() { + for (NamespaceDescriptorImpl descriptor : context.getNamespaceDescriptors().values()) { + Multimap simpleNameDescriptors = descriptor.getMemberScope().getDeclaredDescriptorsAccessibleBySimpleName(); + for (String name : simpleNameDescriptors.keySet()) { + // Keep only properties with no receiver + Collection descriptors = Collections2.filter(simpleNameDescriptors.get(name), new Predicate() { + @Override + public boolean apply(@Nullable DeclarationDescriptor descriptor) { + if (descriptor instanceof PropertyDescriptor) { + PropertyDescriptor propertyDescriptor = (PropertyDescriptor)descriptor; + return !propertyDescriptor.getReceiverParameter().exists(); + } + return true; + } + }); + if (descriptors.size() > 1) { + for (DeclarationDescriptor declarationDescriptor : descriptors) { + for (PsiElement declaration : getDeclarationsByDescriptor(declarationDescriptor)) { + assert declaration != null; + trace.report(REDECLARATION.on(declaration, declarationDescriptor.getName())); + } + } + } + } + } + } + + private Collection getDeclarationsByDescriptor(DeclarationDescriptor declarationDescriptor) { + Collection declarations; + if (declarationDescriptor instanceof NamespaceDescriptor) { + final NamespaceDescriptor namespace = (NamespaceDescriptor)declarationDescriptor; + Collection files = trace.get(BindingContext.NAMESPACE_TO_FILES, namespace); + declarations = Collections2.transform(files, new Function() { + @Override + public PsiElement apply(@Nullable JetFile file) { + assert file != null : "File is null for namespace " + namespace; + return file.getNamespaceHeader().getNameIdentifier(); + } + }); + } + else { + declarations = Collections.singletonList(trace.get(BindingContext.DESCRIPTOR_TO_DECLARATION, declarationDescriptor)); + } + return declarations; + } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/NamespaceFactoryImpl.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/NamespaceFactoryImpl.java index 2abde869b54..d34ae5ed5ca 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/NamespaceFactoryImpl.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/NamespaceFactoryImpl.java @@ -16,6 +16,7 @@ package org.jetbrains.jet.lang.resolve; +import com.google.common.collect.Sets; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.ModuleConfiguration; @@ -28,15 +29,15 @@ import org.jetbrains.jet.lang.psi.JetNamespaceHeader; import org.jetbrains.jet.lang.psi.JetPsiUtil; import org.jetbrains.jet.lang.psi.JetSimpleNameExpression; import org.jetbrains.jet.lang.resolve.scopes.JetScope; +import org.jetbrains.jet.lang.resolve.scopes.RedeclarationHandler; import org.jetbrains.jet.lang.resolve.scopes.WritableScope; import org.jetbrains.jet.lang.resolve.scopes.WritableScopeImpl; import javax.inject.Inject; +import java.util.Collection; import java.util.Collections; -import static org.jetbrains.jet.lang.resolve.BindingContext.FQNAME_TO_NAMESPACE_DESCRIPTOR; -import static org.jetbrains.jet.lang.resolve.BindingContext.REFERENCE_TARGET; -import static org.jetbrains.jet.lang.resolve.BindingContext.RESOLUTION_SCOPE; +import static org.jetbrains.jet.lang.resolve.BindingContext.*; /** * @author Stepan Koltsov @@ -112,7 +113,6 @@ public class NamespaceFactoryImpl implements NamespaceFactory { @NotNull public NamespaceDescriptorImpl createNamespaceDescriptorIfNeeded(@Nullable JetFile file, @NotNull NamespaceDescriptorParent owner, @NotNull String name, boolean root) { - FqName fqName; NamespaceDescriptorImpl namespaceDescriptor; if (root) { @@ -135,7 +135,7 @@ public class NamespaceFactoryImpl implements NamespaceFactory { name ); trace.record(FQNAME_TO_NAMESPACE_DESCRIPTOR, fqName, namespaceDescriptor); - WritableScopeImpl scope = new WritableScopeImpl(JetScope.EMPTY, namespaceDescriptor, new TraceBasedRedeclarationHandler(trace)).setDebugName("Namespace member scope"); + WritableScopeImpl scope = new WritableScopeImpl(JetScope.EMPTY, namespaceDescriptor, RedeclarationHandler.DO_NOTHING).setDebugName("Namespace member scope"); scope.changeLockLevel(WritableScope.LockLevel.BOTH); namespaceDescriptor.initialize(scope); configuration.extendNamespaceScope(trace, namespaceDescriptor, scope); @@ -147,6 +147,15 @@ public class NamespaceFactoryImpl implements NamespaceFactory { if (file != null) { trace.record(BindingContext.FILE_TO_NAMESPACE, file, namespaceDescriptor); + + // Register files corresponding to this namespace + // The trace currently does not support bi-di multimaps that would handle this task nicer + Collection files = trace.get(NAMESPACE_TO_FILES, namespaceDescriptor); + if (files == null) { + files = Sets.newIdentityHashSet(); + } + files.add(file); + trace.record(BindingContext.NAMESPACE_TO_FILES, namespaceDescriptor, files); } return namespaceDescriptor; diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TraceBasedRedeclarationHandler.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TraceBasedRedeclarationHandler.java index 5785064d4fd..0e31d4027f7 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TraceBasedRedeclarationHandler.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TraceBasedRedeclarationHandler.java @@ -39,13 +39,13 @@ public class TraceBasedRedeclarationHandler implements RedeclarationHandler { report(second); } - private void report(DeclarationDescriptor first) { - PsiElement firstElement = trace.get(BindingContext.DESCRIPTOR_TO_DECLARATION, first); + private void report(DeclarationDescriptor descriptor) { + PsiElement firstElement = trace.get(BindingContext.DESCRIPTOR_TO_DECLARATION, descriptor); if (firstElement != null) { - trace.report(REDECLARATION.on(firstElement, first.getName())); + trace.report(REDECLARATION.on(firstElement, descriptor.getName())); } else { - trace.report(REDECLARATION.on(first, trace.getBindingContext())); + throw new IllegalStateException("No declaration found found for " + descriptor); } } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WritableScope.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WritableScope.java index cea1bef5836..e0afdc3407a 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WritableScope.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WritableScope.java @@ -16,6 +16,7 @@ package org.jetbrains.jet.lang.resolve.scopes; +import com.google.common.collect.Multimap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.*; @@ -60,6 +61,9 @@ public interface WritableScope extends JetScope { @Nullable NamespaceDescriptor getDeclaredNamespace(@NotNull String name); + @NotNull + Multimap getDeclaredDescriptorsAccessibleBySimpleName(); + void importScope(@NotNull JetScope imported); void setImplicitReceiver(@NotNull ReceiverDescriptor implicitReceiver); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WritableScopeImpl.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WritableScopeImpl.java index bf2c3d0d5fd..cc871527ce7 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WritableScopeImpl.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WritableScopeImpl.java @@ -16,10 +16,7 @@ package org.jetbrains.jet.lang.resolve.scopes; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; -import com.google.common.collect.SetMultimap; -import com.google.common.collect.Sets; +import com.google.common.collect.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.*; @@ -36,6 +33,7 @@ import java.util.*; public class WritableScopeImpl extends WritableScopeWithImports { private final Collection allDescriptors = Sets.newLinkedHashSet(); + private final Multimap declaredDescriptorsAccessibleBySimpleName = HashMultimap.create(); private boolean allDescriptorsDone = false; @NotNull @@ -226,6 +224,7 @@ public class WritableScopeImpl extends WritableScopeWithImports { getVariableClassOrNamespaceDescriptors().put(name, variableDescriptor); } allDescriptors.add(variableDescriptor); + addToDeclared(variableDescriptor); } @NotNull @@ -334,6 +333,7 @@ public class WritableScopeImpl extends WritableScopeWithImports { checkForRedeclaration(name, classifierDescriptor); getVariableClassOrNamespaceDescriptors().put(name, classifierDescriptor); allDescriptors.add(classifierDescriptor); + addToDeclared(classifierDescriptor); } @Override @@ -343,6 +343,7 @@ public class WritableScopeImpl extends WritableScopeWithImports { checkForRedeclaration(name, namespaceDescriptor); getNamespaceAliases().put(name, namespaceDescriptor); allDescriptors.add(namespaceDescriptor); + addToDeclared(namespaceDescriptor); } @Override @@ -361,6 +362,7 @@ public class WritableScopeImpl extends WritableScopeWithImports { checkForRedeclaration(name, variableDescriptor); getVariableClassOrNamespaceDescriptors().put(name, variableDescriptor); allDescriptors.add(variableDescriptor); + addToDeclared(variableDescriptor); } private void checkForPropertyRedeclaration(String name, VariableDescriptor variableDescriptor) { @@ -417,6 +419,7 @@ public class WritableScopeImpl extends WritableScopeWithImports { redeclarationHandler.handleRedeclaration(oldValue, namespaceDescriptor); } allDescriptors.add(namespaceDescriptor); + addToDeclared(namespaceDescriptor); } @Override @@ -521,4 +524,14 @@ public class WritableScopeImpl extends WritableScopeWithImports { super.setDebugName(debugName); return this; } + + private void addToDeclared(DeclarationDescriptor descriptor) { + declaredDescriptorsAccessibleBySimpleName.put(descriptor.getName(), descriptor); + } + + @NotNull + @Override + public Multimap getDeclaredDescriptorsAccessibleBySimpleName() { + return declaredDescriptorsAccessibleBySimpleName; + } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WriteThroughScope.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WriteThroughScope.java index cf80f668075..08380c75cbb 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WriteThroughScope.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/scopes/WriteThroughScope.java @@ -17,6 +17,7 @@ package org.jetbrains.jet.lang.resolve.scopes; import com.google.common.collect.Lists; +import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -257,6 +258,12 @@ public class WriteThroughScope extends WritableScopeWithImports { return writableWorker.getDeclaredNamespace(name); } + @NotNull + @Override + public Multimap getDeclaredDescriptorsAccessibleBySimpleName() { + return writableWorker.getDeclaredDescriptorsAccessibleBySimpleName(); + } + @Override public void importScope(@NotNull JetScope imported) { checkMayWrite(); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingVisitor.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingVisitor.java index b8d7fc3f06d..868a4acf3f5 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingVisitor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingVisitor.java @@ -270,7 +270,8 @@ public class ControlStructureTypingVisitor extends ExpressionTypingVisitor { VariableDescriptor olderVariable = context.scope.getLocalVariable(variableDescriptor.getName()); if (olderVariable != null && DescriptorUtils.isLocal(context.scope.getContainingDeclaration(), olderVariable)) { - context.trace.report(Errors.NAME_SHADOWING.on(variableDescriptor, context.trace.getBindingContext())); + PsiElement declaration = context.trace.get(DESCRIPTOR_TO_DECLARATION, variableDescriptor); + context.trace.report(Errors.NAME_SHADOWING.on(declaration, variableDescriptor.getName())); } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ExpressionTypingVisitorForStatements.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ExpressionTypingVisitorForStatements.java index 066cfa47c0a..0d1774e4e5c 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ExpressionTypingVisitorForStatements.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ExpressionTypingVisitorForStatements.java @@ -17,6 +17,7 @@ package org.jetbrains.jet.lang.types.expressions; import com.google.common.collect.Sets; +import com.intellij.psi.PsiElement; import com.intellij.psi.tree.IElementType; import com.intellij.util.ObjectUtils; import org.jetbrains.annotations.NotNull; @@ -43,6 +44,7 @@ import java.util.Collection; import static org.jetbrains.jet.lang.diagnostics.Errors.*; import static org.jetbrains.jet.lang.resolve.BindingContext.AMBIGUOUS_REFERENCE_TARGET; +import static org.jetbrains.jet.lang.resolve.BindingContext.DESCRIPTOR_TO_DECLARATION; import static org.jetbrains.jet.lang.resolve.BindingContext.VARIABLE_REASSIGNMENT; /** @@ -116,7 +118,8 @@ public class ExpressionTypingVisitorForStatements extends ExpressionTypingVisito { VariableDescriptor olderVariable = scope.getLocalVariable(propertyDescriptor.getName()); if (olderVariable != null && DescriptorUtils.isLocal(propertyDescriptor.getContainingDeclaration(), olderVariable)) { - context.trace.report(Errors.NAME_SHADOWING.on(propertyDescriptor, context.trace.getBindingContext())); + PsiElement declaration = context.trace.get(DESCRIPTOR_TO_DECLARATION, propertyDescriptor); + context.trace.report(Errors.NAME_SHADOWING.on(declaration, propertyDescriptor.getName())); } } diff --git a/compiler/testData/diagnostics/tests/redeclarations/MultiFilePackageRedeclaration.jet b/compiler/testData/diagnostics/tests/redeclarations/MultiFilePackageRedeclaration.jet new file mode 100644 index 00000000000..40ac213b622 --- /dev/null +++ b/compiler/testData/diagnostics/tests/redeclarations/MultiFilePackageRedeclaration.jet @@ -0,0 +1,7 @@ +// FILE: f.kt +package a +class b {} +// FILE: f.kt +package a.b +// FILE: f.kt +package a.b \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/Redeclarations.jet b/compiler/testData/diagnostics/tests/redeclarations/Redeclarations.jet similarity index 61% rename from compiler/testData/diagnostics/tests/Redeclarations.jet rename to compiler/testData/diagnostics/tests/redeclarations/Redeclarations.jet index 34b0dcd2650..5bde0b1df84 100644 --- a/compiler/testData/diagnostics/tests/Redeclarations.jet +++ b/compiler/testData/diagnostics/tests/redeclarations/Redeclarations.jet @@ -1,6 +1,6 @@ // FILE: f.kt package redeclarations - object A { + object A { val x : Int = 0 val A = 1 @@ -11,5 +11,5 @@ package redeclarations val A = 1 // FILE: f.kt - package redeclarations.A + package redeclarations.A class A {}