Skip to content
Merged
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
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.sonar.plugins.communitydelphi.api.symbol.scope.FileScope;

public interface UnitImportNameDeclaration extends QualifiedNameDeclaration {
boolean isAlias();

@Nullable
UnitNameDeclaration getOriginalDeclaration();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
Loading