Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix type detection of nested annotations #3604

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion buildScripts/vm-finder.ant.xml
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ and rerun the build; this build is capable of finding VMs automatically on many
<matches pattern="^\s*$" string="${jvm.loc}" />
</condition>
<fail if="jvm.loc.aborted">aborted</fail>
<fail if="jvm.loc.invalid">.
<fail>.

ERROR: That does not appear to be a valid location; ${jvm.loc}/bin/${exe.java} should exist.
<condition>
Expand Down
105 changes: 98 additions & 7 deletions src/core/lombok/core/TypeResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
*/
package lombok.core;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import lombok.core.AST.Kind;
Expand All @@ -32,7 +34,7 @@
* and this importer also can't find inner types from superclasses/interfaces.
*/
public class TypeResolver {
private ImportList imports;
private final ImportList imports;

/**
* Creates a new TypeResolver that can be used to resolve types in a source file with the given package and import statements.
Expand All @@ -53,6 +55,14 @@ public String typeRefToFullyQualifiedName(LombokNode<?, ?, ?> context, TypeLibra
// When asking if 'lombok.Getter' could possibly be referring to 'lombok.Getter', the answer is obviously yes.
if (qualifieds.contains(typeRef)) return LombokInternalAliasing.processAliases(typeRef);

// Types defined on the containing type, or on any of its parents in the source file, take precedence over any imports
String nestedTypeFqn = new NestedTypeFinder(typeRef).findNestedType(context);
if (nestedTypeFqn != null) {
// we found a nestedType - check edge case where nestedType is in type library
qualifieds = library.toQualifieds(nestedTypeFqn);
return qualifieds == null || !qualifieds.contains(nestedTypeFqn) ? null : nestedTypeFqn;
}

// When asking if 'Getter' could possibly be referring to 'lombok.Getter' if 'import lombok.Getter;' is in the source file, the answer is yes.
int firstDot = typeRef.indexOf('.');
if (firstDot == -1) firstDot = typeRef.length();
Expand Down Expand Up @@ -96,12 +106,7 @@ public String typeRefToFullyQualifiedName(LombokNode<?, ?, ?> context, TypeLibra
continue mainLoop;
}

if (n.getKind() == Kind.TYPE || n.getKind() == Kind.COMPILATION_UNIT) {
for (LombokNode<?, ?, ?> child : n.down()) {
// Inner class that's visible to us has 'typeRef' as name, so that's the one being referred to, not one of our type library classes.
if (child.getKind() == Kind.TYPE && firstTypeRef.equals(child.getName())) return null;
}
}
// don't need to check for inner class shadowing, we already do that in NestedTypeFinder

n = n.directUp();
}
Expand All @@ -113,4 +118,90 @@ public String typeRefToFullyQualifiedName(LombokNode<?, ?, ?> context, TypeLibra
// No star import matches either.
return null;
}

/**
* Traverse up the containing types until we find a match, or hit the package. At each level,
* we check for a type with matching name (including traversing into child types if typeRef is
* not a simple name).
*/
private static class NestedTypeFinder {

private final String typeRef;
private final List<String> typeRefElements;

public NestedTypeFinder(String typeRef) {
this.typeRef = typeRef;
this.typeRefElements = Arrays.asList(typeRef.split("\\.", -1));
}

/** Finds a matching nestedType and returns its FQN, or {@code null} if no match found. */
public String findNestedType(LombokNode<?, ?, ?> context) {
LombokNode<?, ?, ?> nearestType = traverseUpToNearestType(context);
if (nearestType == null) {
return null;
}

boolean found = findTypeRef(nearestType, 0);
if (found) {
// return FQN
return getFoundFqn(nearestType);
}

return findNestedType(nearestType.up());
}

/** Traverse up to the nearest type or package (including {@code node} if it is a type). */
private LombokNode<?, ?, ?> traverseUpToNearestType(LombokNode<?, ?, ?> node) {
if (node == null) {
return null; // parent is null once we hit the package
}
if (node.getKind() == Kind.COMPILATION_UNIT || node.getKind() == Kind.TYPE) {
return node;
}
return traverseUpToNearestType(node.up());
}

/** Check whether {@code typeRef[nameIndex]} exists as a child of {@code typeNode}. */
private boolean findTypeRef(LombokNode<?, ?, ?> typeNode, int nameIndex) {
for (LombokNode<?, ?, ?> child : typeNode.down()) {
if (child.getKind() == Kind.TYPE) {
// check if this node matches the first element
if (child.getName().equals(typeRefElements.get(nameIndex))) {
if (nameIndex == typeRefElements.size() - 1) {
// we've found a match as we've matched all elements of typeRef
return true;
}
// otherwise, check match of remaining typeRef elements
boolean found = findTypeRef(child, nameIndex + 1);
if (found) {
return true;
}
}
}
}
return false;
}

private String getFoundFqn(LombokNode<?, ?, ?> typeNode) {
List<String> elements = new ArrayList<String>();
while (typeNode.getKind() != Kind.COMPILATION_UNIT) {
elements.add(typeNode.getName());
typeNode = traverseUpToNearestType(typeNode.up());
}

String pkg = typeNode.getPackageDeclaration();
StringBuilder fqn;
if (pkg == null) { // pkg can be null e.g. if top-level type is in default package
fqn = new StringBuilder(elements.size() * 10);
} else {
fqn = new StringBuilder(pkg.length() + elements.size() * 10);
fqn.append(pkg).append('.');
}
for (int i = elements.size() - 1; i >= 0; i--) {
fqn.append(elements.get(i)).append('.');
}
fqn.append(typeRef);
return fqn.toString();
}
}
}
11 changes: 11 additions & 0 deletions test/stubs/test/lombok/other/Other.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package test.lombok.other;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

public class Other {

@Retention(RetentionPolicy.RUNTIME)
public @interface TA {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package test.lombok;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import test.lombok.NestedClassAndAnnotationNestedInImportedSibling.Other.OtherNested.TA;

public class NestedClassAndAnnotationNestedInImportedSibling {

public static class Inner {

@TA
private final int someVal;

@java.lang.SuppressWarnings("all")
public Inner(@TA final int someVal) {
this.someVal = someVal;
}

@TA
@java.lang.SuppressWarnings("all")
public int getSomeVal() {
return this.someVal;
}
}

public static class Other {

public static class OtherNested {

@Retention(RetentionPolicy.RUNTIME)
public @interface TA {
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package test.lombok;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

public class NestedClassAndAnnotationNestedInNonImportedSibling {

public static class Inner {
@Other.OtherNested.TA
private final int someVal;

@java.lang.SuppressWarnings("all")
public Inner(@Other.OtherNested.TA final int someVal) {
this.someVal = someVal;
}

@Other.OtherNested.TA
@java.lang.SuppressWarnings("all")
public int getSomeVal() {
return this.someVal;
}
}

public static class Other {

public static class OtherNested {
@Retention(RetentionPolicy.RUNTIME)
public @interface TA {
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package test.lombok;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

public class NestedClassAndNestedAnnotation {

public static class Inner {
@TA
private final int someVal;

@java.lang.SuppressWarnings("all")
public Inner(@TA final int someVal) {
this.someVal = someVal;
}

@TA
@java.lang.SuppressWarnings("all")
public int getSomeVal() {
return this.someVal;
}
}

@Retention(RetentionPolicy.RUNTIME)
public @interface TA {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package test.lombok;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import test.lombok.NestedClassAndNestedAnnotationImported.Other.OtherNested.TA;

public class NestedClassAndNestedAnnotationImported {

public static class Inner {
@TA
private final int someVal;

@java.lang.SuppressWarnings("all")
public Inner(@TA final int someVal) {
this.someVal = someVal;
}

@TA
@java.lang.SuppressWarnings("all")
public int getSomeVal() {
return this.someVal;
}
}

public static class Other {

public static class OtherNested {
@Retention(RetentionPolicy.RUNTIME)
public @interface TA {
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package test.lombok;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import test.lombok.other.Other;

public class NestedClassAndNestedAnnotationWithConflictingNestedImport {

public static class Inner {
@Other.TA
private final int someVal;

@java.lang.SuppressWarnings("all")
public Inner(@Other.TA final int someVal) {
this.someVal = someVal;
}

@Other.TA
@java.lang.SuppressWarnings("all")
public int getSomeVal() {
return this.someVal;
}
}

public static class Other {

@Retention(RetentionPolicy.RUNTIME)
public @interface TA {
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package test.lombok;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import test.lombok.other.Other.TA;

public class NestedClassAndNestedAnnotationWithOverridingNestedAnnotationImport {

public static class Inner {
@TA
private final int someVal;

@java.lang.SuppressWarnings("all")
public Inner(@TA final int someVal) {
this.someVal = someVal;
}

@TA
@java.lang.SuppressWarnings("all")
public int getSomeVal() {
return this.someVal;
}
}

public static class Other {

@Retention(RetentionPolicy.RUNTIME)
public @interface TA {
}
}
}
25 changes: 25 additions & 0 deletions test/transform/resource/after-delombok/SimpleNestedAnnotation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package test.lombok;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

public class SimpleNestedAnnotation {

@TA
private final int someVal;

@Retention(RetentionPolicy.RUNTIME)
public @interface TA {
}

@java.lang.SuppressWarnings("all")
public SimpleNestedAnnotation(@TA final int someVal) {
this.someVal = someVal;
}

@TA
@java.lang.SuppressWarnings("all")
public int getSomeVal() {
return this.someVal;
}
}
Loading