diff --git a/CHANGELOG.md b/CHANGELOG.md index 4308ae436..c6463c192 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `FullyQualifiedImportCheck` analysis rule, which flags non-fully qualified imports. +- **API:** `UnitImportNameDeclaration::isAlias` method. + ### Changed - Non-trivial `inherited` expressions are excluded in `RedundantParentheses`. @@ -19,7 +24,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `FullyQualifiedImportCheck` analysis rule, which flags non-fully qualified imports. - **API:** `CaseItemStatementNode::getExpressions` method. ### Changed diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/FullyQualifiedImportCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/FullyQualifiedImportCheck.java index 15a2b27f9..3f1ab572b 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/FullyQualifiedImportCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/FullyQualifiedImportCheck.java @@ -25,32 +25,32 @@ import org.sonar.plugins.communitydelphi.api.reporting.QuickFix; import org.sonar.plugins.communitydelphi.api.reporting.QuickFixEdit; import org.sonar.plugins.communitydelphi.api.symbol.declaration.UnitImportNameDeclaration; +import org.sonar.plugins.communitydelphi.api.symbol.declaration.UnitNameDeclaration; @Rule(key = "FullyQualifiedImport") public class FullyQualifiedImportCheck extends DelphiCheck { @Override public DelphiCheckContext visit(UnitImportNode unitImportNode, DelphiCheckContext context) { - if (!unitImportNode.isResolvedImport()) { + UnitImportNameDeclaration importDeclaration = unitImportNode.getImportNameDeclaration(); + UnitNameDeclaration unitDeclaration = importDeclaration.getOriginalDeclaration(); + + if (unitDeclaration == null || importDeclaration.isAlias()) { return context; } - UnitImportNameDeclaration unitImportNameDeclaration = unitImportNode.getImportNameDeclaration(); - - String unitFullyQualifiedName = unitImportNameDeclaration.getOriginalDeclaration().getImage(); - String unitImportName = unitImportNameDeclaration.getImage(); + String actual = importDeclaration.fullyQualifiedName(); + String expected = unitDeclaration.fullyQualifiedName(); - if (unitImportName.length() != unitFullyQualifiedName.length()) { + if (!actual.equalsIgnoreCase(expected)) { context .newIssue() .onNode(unitImportNode) .withMessage( - "Fully qualify this unit name (found: \"%s\" expected: \"%s\").", - unitImportName, unitFullyQualifiedName) + "Fully qualify this unit name (found: \"%s\" expected: \"%s\").", actual, expected) .withQuickFixes( QuickFix.newFix("Fully qualify unit import") - .withEdit( - QuickFixEdit.replace(unitImportNode.getNameNode(), unitFullyQualifiedName))) + .withEdit(QuickFixEdit.replace(unitImportNode.getNameNode(), expected))) .report(); } diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/FullyQualifiedImportCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/FullyQualifiedImportCheckTest.java index 20884bca0..acf13adfd 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/FullyQualifiedImportCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/FullyQualifiedImportCheckTest.java @@ -82,6 +82,33 @@ void testUnresolvedImportShouldNotAddIssue() { .verifyNoIssues(); } + @Test + void testUnitAliasImportShouldNotAddIssue() { + var testFile = new DelphiTestUnitBuilder().appendDecl("uses").appendDecl(" AliasName;"); + + CheckVerifier.newVerifier() + .withCheck(new FullyQualifiedImportCheck()) + .withSearchPathUnit(new DelphiTestUnitBuilder().unitName("Scope.RealName")) + .withSearchPathUnit(new DelphiTestUnitBuilder().unitName("Scope.AliasName")) + .withUnitScopeName("Scope") + .withUnitAlias("AliasName", "Scope.RealName") + .onFile(testFile) + .verifyNoIssues(); + } + + @Test + void testUnitAliasImportThatLooksLikeUnqualifiedImportShouldNotAddIssue() { + var testFile = new DelphiTestUnitBuilder().appendDecl("uses").appendDecl(" Name;"); + + CheckVerifier.newVerifier() + .withCheck(new FullyQualifiedImportCheck()) + .withSearchPathUnit(new DelphiTestUnitBuilder().unitName("Scope.Name")) + .withUnitScopeName("Scope") + .withUnitAlias("Name", "Scope.Name") + .onFile(testFile) + .verifyNoIssues(); + } + @Test void testNotFullyQualifiedImportShouldAddIssue() { var importedUnit = new DelphiTestUnitBuilder().unitName("Scope.UnitU"); diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/SymbolTableBuilder.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/SymbolTableBuilder.java index 2c59decea..702469d08 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/SymbolTableBuilder.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/SymbolTableBuilder.java @@ -50,8 +50,8 @@ import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.sonar.plugins.communitydelphi.api.ast.QualifiedNameDeclarationNode; import org.sonar.plugins.communitydelphi.api.ast.UnitImportNode; -import org.sonar.plugins.communitydelphi.api.symbol.Qualifiable; import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration; import org.sonar.plugins.communitydelphi.api.symbol.declaration.RoutineDirective; import org.sonar.plugins.communitydelphi.api.symbol.declaration.TypeNameDeclaration; @@ -202,7 +202,17 @@ private void createUnitData(Path unitPath, boolean isSourceFile) { private UnitImportNameDeclaration createImportDeclaration( UnitNameDeclaration unit, UnitImportNode node) { - UnitData data = searchForImport(unit, node.getNameNode()); + QualifiedNameDeclarationNode nameNode = node.getNameNode(); + + String importName = nameNode.fullyQualifiedName(); + String aliased = unitAliases.get(importName); + + boolean alias = aliased != null; + if (alias) { + importName = aliased; + } + + UnitData data = searchForImport(unit, importName, nameNode.isQualified()); UnitNameDeclaration unitDeclaration = null; if (data != null) { @@ -214,18 +224,12 @@ private UnitImportNameDeclaration createImportDeclaration( LOG.debug("{}X {} **Failed to locate unit**", indentation, unitName); } - return new UnitImportNameDeclarationImpl(node, unitDeclaration); + return new UnitImportNameDeclarationImpl(node, alias, unitDeclaration); } @Nullable - private UnitData searchForImport(UnitNameDeclaration unit, Qualifiable qualifiableImportName) { - String importName = qualifiableImportName.fullyQualifiedName(); - String aliased = unitAliases.get(importName); - - if (aliased != null) { - importName = aliased; - } - + private UnitData searchForImport( + UnitNameDeclaration unit, String importName, boolean isQualified) { UnitData data = findImportByName(unit, importName); if (data == null) { @@ -239,7 +243,7 @@ private UnitData searchForImport(UnitNameDeclaration unit, Qualifiable qualifiab if (data == null) { String namespace = unit.getNamespace(); - if (!qualifiableImportName.isQualified() && !namespace.isEmpty()) { + if (!isQualified && !namespace.isEmpty()) { data = findImportByName(unit, namespace + "." + importName); } } diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/declaration/UnitImportNameDeclarationImpl.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/declaration/UnitImportNameDeclarationImpl.java index 4bef572ae..3ea0ecae0 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/declaration/UnitImportNameDeclarationImpl.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/declaration/UnitImportNameDeclarationImpl.java @@ -18,23 +18,36 @@ */ package au.com.integradev.delphi.symbol.declaration; +import static java.util.Comparator.naturalOrder; +import static java.util.Comparator.nullsLast; + +import com.google.common.collect.ComparisonChain; import java.util.Objects; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.sonar.plugins.communitydelphi.api.ast.UnitImportNode; +import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration; import org.sonar.plugins.communitydelphi.api.symbol.declaration.UnitImportNameDeclaration; import org.sonar.plugins.communitydelphi.api.symbol.declaration.UnitNameDeclaration; import org.sonar.plugins.communitydelphi.api.symbol.scope.FileScope; public final class UnitImportNameDeclarationImpl extends QualifiedNameDeclarationImpl implements UnitImportNameDeclaration { + private final boolean alias; private final UnitNameDeclaration originalDeclaration; public UnitImportNameDeclarationImpl( - UnitImportNode node, @Nullable UnitNameDeclaration originalDeclaration) { + UnitImportNode node, boolean alias, @Nullable UnitNameDeclaration originalDeclaration) { super(node.getNameNode()); + this.alias = alias; this.originalDeclaration = originalDeclaration; } + @Override + public boolean isAlias() { + return alias; + } + @Override @Nullable public UnitNameDeclaration getOriginalDeclaration() { @@ -54,14 +67,27 @@ public FileScope getUnitScope() { public boolean equals(Object other) { if (super.equals(other)) { UnitImportNameDeclarationImpl that = (UnitImportNameDeclarationImpl) other; - return Objects.equals(originalDeclaration, that.originalDeclaration); + return alias == that.alias && Objects.equals(originalDeclaration, that.originalDeclaration); } return false; } @Override public int hashCode() { - return Objects.hash(super.hashCode(), originalDeclaration); + return Objects.hash(super.hashCode(), alias, originalDeclaration); + } + + @Override + public int compareTo(@Nonnull NameDeclaration other) { + int result = super.compareTo(other); + if (result == 0) { + UnitImportNameDeclarationImpl that = (UnitImportNameDeclarationImpl) other; + return ComparisonChain.start() + .compareTrueFirst(this.alias, that.alias) + .compare(this.originalDeclaration, that.originalDeclaration, nullsLast(naturalOrder())) + .result(); + } + return result; } @Override diff --git a/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/symbol/declaration/UnitImportNameDeclaration.java b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/symbol/declaration/UnitImportNameDeclaration.java index f8fbd5979..f362b7222 100644 --- a/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/symbol/declaration/UnitImportNameDeclaration.java +++ b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/symbol/declaration/UnitImportNameDeclaration.java @@ -22,6 +22,8 @@ import org.sonar.plugins.communitydelphi.api.symbol.scope.FileScope; public interface UnitImportNameDeclaration extends QualifiedNameDeclaration { + boolean isAlias(); + @Nullable UnitNameDeclaration getOriginalDeclaration(); diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/declaration/UnitImportNameDeclarationTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/declaration/UnitImportNameDeclarationTest.java index 28f23972c..40f5dd340 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/declaration/UnitImportNameDeclarationTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/declaration/UnitImportNameDeclarationTest.java @@ -39,20 +39,32 @@ import org.sonar.plugins.communitydelphi.api.symbol.declaration.UnitNameDeclaration; class UnitImportNameDeclarationTest { + @Test + void testIsAlias() { + assertThat(createImport("Foo", true).isAlias()).isTrue(); + assertThat(createImport("Bar", false).isAlias()).isFalse(); + } + @Test void testEquals() { UnitImportNameDeclaration foo = createImport("Foo"); UnitImportNameDeclaration otherFoo = createImport("Foo"); + UnitImportNameDeclaration aliasFoo = createImport("Foo", true); UnitImportNameDeclaration differentName = createImport("Bar"); UnitImportNameDeclaration differentOriginalDeclaration = createImport("Foo", createUnit("Foo")); new EqualsTester() .addEqualityGroup(foo, otherFoo) + .addEqualityGroup(aliasFoo) .addEqualityGroup(differentName) .addEqualityGroup(differentOriginalDeclaration) .testEquals(); - assertThat(foo).isEqualByComparingTo(otherFoo).isNotEqualByComparingTo(differentName); + assertThat(foo) + .isEqualByComparingTo(otherFoo) + .isNotEqualByComparingTo(aliasFoo) + .isNotEqualByComparingTo(differentName) + .isNotEqualByComparingTo(differentOriginalDeclaration); } @Test @@ -64,11 +76,20 @@ private static UnitImportNameDeclaration createImport(String name) { return createImport(name, null); } + private static UnitImportNameDeclaration createImport(String name, boolean alias) { + return createImport(name, null, alias); + } + private static UnitImportNameDeclaration createImport( String name, UnitNameDeclaration originalDeclaration) { + return createImport(name, originalDeclaration, false); + } + + private static UnitImportNameDeclaration createImport( + String name, UnitNameDeclaration originalDeclaration, boolean alias) { var location = new UnitImportNodeImpl(DelphiLexer.TkUnitImport); location.addChild(createNameNode(name)); - return new UnitImportNameDeclarationImpl(location, originalDeclaration); + return new UnitImportNameDeclarationImpl(location, alias, originalDeclaration); } private static UnitNameDeclaration createUnit(String name) {