Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,13 @@ integration, telemetry connectivity, and Azure Toolkit integration.
to
the [Troubleshoot dependency version conflicts documentation](https://learn.microsoft.com/en-us/azure/developer/java/sdk/troubleshooting-dependency-version-conflict)
for additional information on resolving dependency version conflicts.


17. ## Using Implementation Types instead of Publicly Available Azure Classes

**Anti-pattern**: Using implementation types instead of publicly available Azure classes.
- **Issue**: Implementation types are internal classes that are not intended for public use. They may change or be
removed
in future versions, leading to compatibility issues.
- **Severity: ERROR**
- **Recommendation**: Use the publicly available Azure classes instead.
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool;

import com.intellij.codeInspection.LocalInspectionTool;
import com.intellij.codeInspection.ProblemsHolder;
import com.intellij.psi.JavaElementVisitor;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiClassType;
import com.intellij.psi.PsiType;
import com.intellij.psi.PsiVariable;
import org.jetbrains.annotations.NotNull;

/**
* This class is an inspection tool that checks if a variable type is an Azure implementation type.
* If the variable type is an Azure implementation type, or if the variable type extends or implements an Azure implementation type,
* the inspection tool will flag it as a problem.
*/
public class ImplementationTypeCheck extends LocalInspectionTool {

/**
* Build the visitor for the inspection. This visitor will be used to traverse the PSI tree.
*
* @param holder The holder for the problems found
* @param isOnTheFly Whether the inspection is being run on the fly - This is not used in this implementation, but is required by the interface
* @return The visitor for the inspection
*/
@NotNull
@Override
public JavaElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
return new ImplementationTypeVisitor(holder);
}

/**
* This class extends the JavaElementVisitor to visit the elements in the code.
* It checks if the variable type is an Azure implementation type and flags it as a problem if it is.
*/
static class ImplementationTypeVisitor extends JavaElementVisitor {

private final ProblemsHolder holder;

/**
* Constructor for the visitor
*
* @param holder - the ProblemsHolder object to register the problem
*/
ImplementationTypeVisitor(ProblemsHolder holder) {
this.holder = holder;
}

// Define constants for string literals
private static final RuleConfig RULE_CONFIG;
private static final boolean SKIP_WHOLE_RULE;
private static final String RULE_NAME = "ImplementationTypeCheck";

static {
RuleConfigLoader centralRuleConfigLoader = RuleConfigLoader.getInstance();

// Get the RuleConfig object for the rule
RULE_CONFIG = centralRuleConfigLoader.getRuleConfig(RULE_NAME);
SKIP_WHOLE_RULE = RULE_CONFIG.skipRuleCheck() || RULE_CONFIG.getAntiPatternMessageMap().isEmpty() || RULE_CONFIG.getListedItemsToCheck().isEmpty();
}

/**
* This method visits the variables in the code.
* It checks if the variable type is an Azure implementation type and flags it as a problem if it is.
*/
@Override
public void visitVariable(@NotNull PsiVariable variable) {
super.visitVariable(variable);

if (SKIP_WHOLE_RULE) {
return;
}

// Get the type of the variable - This is the type of the variable declaration
// eg. List<String> myList = new ArrayList<>(); -> type = List<String>
PsiType type = variable.getType();
PsiClass psiClass = ((PsiClassType) type).resolve();

// Check if the type directly used is an implementation type
if (isImplementationType(psiClass) && variable.getNameIdentifier() != null) {
holder.registerProblem(variable.getNameIdentifier(), RULE_CONFIG.getAntiPatternMessageMap().get("antiPatternMessage"));

// Check if the type extends or implements an implementation type
} else if (extendsOrImplementsImplementationType(psiClass) && variable.getNameIdentifier() != null) {
holder.registerProblem(variable.getNameIdentifier(), RULE_CONFIG.getAntiPatternMessageMap().get("antiPatternMessage"));
}
}

/**
* This method checks if the type is a class from the Azure package and if it is an implementation type.
* It returns true if the type is an implementation type and false otherwise.
*/
private boolean isImplementationType(PsiClass psiClass) {

if (psiClass == null) {
return false;
}

for (String listedItem : RULE_CONFIG.getListedItemsToCheck()) {
if (psiClass.getQualifiedName() != null) {

// Check if the class is in the Azure package and if it is an implementation type
return psiClass.getQualifiedName().startsWith(RuleConfig.AZURE_PACKAGE_NAME) && psiClass.getQualifiedName().contains(listedItem);
}
}
return false;
}

/**
* This method checks if the type is a class that extends or implements an implementation type.
* It returns true if the type extends or implements an implementation type and false otherwise.
*/
private boolean extendsOrImplementsImplementationType(PsiClass psiClass) {

// If the class is null or if it is a Java class, it is not an implementation type nor does it extend or implement one
if (psiClass == null || psiClass.getQualifiedName().startsWith("java.")) {
return false;
}

// Check if the current class is an implementation type
if (isImplementationType(psiClass)) {
return true;
}

// Check all direct interfaces
for (PsiClass iface : psiClass.getInterfaces()) {
if (extendsOrImplementsImplementationType(iface)) {
return true;
}
}
// Check the direct superclass
PsiClass superClass = psiClass.getSuperClass();
if (superClass != null) {
return extendsOrImplementsImplementationType(superClass);
}
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ private RuleConfig getRuleConfig(JsonReader reader) throws IOException {
case "url":
listedItemsToCheck = getListFromJsonArray(reader);
break;
case "subPackageName":
listedItemsToCheck = getListFromJsonArray(reader);
break;
default:
if (fieldName.endsWith("Check")) {
// Move to the next token to process the nested object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,5 +261,19 @@
com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool.IncompatibleDependencyCheck
</class>
</localInspection>

<localInspection
language="JAVA"
shortName="ImplementationTypeCheck"
displayName="Implementation Type Check"
groupName="Azure SDK"
enabledByDefault="true"
level="ERROR"
implementationClass="com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool.ImplementationTypeCheck">

<class>
com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool.ImplementationTypeCheck
</class>
</localInspection>
</extensions>
</idea-plugin>
Original file line number Diff line number Diff line change
Expand Up @@ -143,5 +143,9 @@
"IncompatibleDependencyCheck": {
"url": "https://raw.githubusercontent.com/Azure/azure-sdk-for-java/main/eng/versioning/supported_external_dependency_versions.json",
"antiPatternMessage": "The version of {{fullName}} is not compatible with other dependencies of the same library defined in the pom.xml. Please use versions of the same library release group {{recommendedVersion}}.x to ensure proper functionality."
},
"ImplementationTypeCheck": {
"subPackageName": "implementation",
"antiPatternMessage": "Detected usage of an implementation type. Implementation types are not intended for public use. Use the publicly available Azure classes instead."
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
package com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool;

import com.intellij.codeInspection.ProblemsHolder;
import com.intellij.psi.JavaElementVisitor;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiClassType;
import com.intellij.psi.PsiIdentifier;
import com.intellij.psi.PsiVariable;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;

import com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool.ImplementationTypeCheck.ImplementationTypeVisitor;
import org.mockito.Mockito;

import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

/**
* This class tests the ImplementationTypeVisitor class.
*/
public class ImplementationTypeCheckTest {

// Declare as instance variables
@Mock
private ProblemsHolder mockHolder;

@Mock
private JavaElementVisitor mockVisitor;

@Mock
private PsiVariable mockVariable;

@BeforeEach
public void setup() {
mockHolder = mock(ProblemsHolder.class);
mockVisitor = createVisitor();
mockVariable = mock(PsiVariable.class);
}

/**
* Test cases for the ImplementationTypeVisitor class.
* This case is for a class that is an implementation type.
* The class is in the implementation package.
* The registerProblem method should be called.
*/
@Test
public void testDirectUseOfImplementationType() {
String classQualifiedName = "com.azure.data.appconfiguration.implementation.models";

// No interfaces implemented
PsiClass[] interfaces = new PsiClass[]{};
String interfaceQualifiedName = null;

// Extends Object (implicitly)
PsiClass superClass = mock(PsiClass.class);
String superClassQualifiedName = "java.lang.Object"; // Extends java.lang.Object

int numOfInvocations = 1;
verifyRegisterProblem(classQualifiedName, interfaces, superClass, superClassQualifiedName, interfaceQualifiedName, numOfInvocations);
}

/**
* Test cases for the ImplementationTypeVisitor class.
* This case is for a class that is not an implementation type.
* The registerProblem method should not be called.
*/
@Test
public void testUseOfNonImplementationAzurePackage() {
String classQualifiedName = "com.azure.data.appconfiguration.models";

// No interfaces implemented
PsiClass[] interfaces = new PsiClass[]{};
String interfaceQualifiedName = null;

// Extends Object (implicitly)
PsiClass superClass = mock(PsiClass.class);
String superClassQualifiedName = "java.lang.Object"; // Extends java.lang.Object

int numOfInvocations = 0;
verifyRegisterProblem(classQualifiedName, interfaces, superClass, superClassQualifiedName, interfaceQualifiedName, numOfInvocations);
}

/**
* Test cases for the ImplementationTypeVisitor class.
* This case is for a class that implements an implementation type interface.
* The registerProblem method should be called.
*/
@Test
public void testUseOfImplementationTypeInterface() {
String classQualifiedName = "com.azure.data.appconfiguration.models";

// Implements an implementation type interface
PsiClass interfaceClass = mock(PsiClass.class);
String interfaceQualifiedName = "com.azure.data.appconfiguration.implementation.models";
PsiClass[] interfaces = new PsiClass[]{interfaceClass};

// Extends Object (implicitly)
PsiClass superClass = mock(PsiClass.class);
String superClassQualifiedName = "java.lang.Object"; // Extends java.lang.Object

int numOfInvocations = 1;
verifyRegisterProblem(classQualifiedName, interfaces, superClass, superClassQualifiedName, interfaceQualifiedName, numOfInvocations);
}

/**
* Test cases for the ImplementationTypeVisitor class.
* This case is for a class that extends an implementation type abstract class.
* The registerProblem method should be called.
*/
@Test
public void testUseOfImplementationTypeAbstractClass() {
String classQualifiedName = "com.azure.data.appconfiguration.models";

// Does not implement any interface
String interfaceQualifiedName = null;
PsiClass[] interfaces = new PsiClass[]{};

// Extends Object (implicitly)
PsiClass superClass = mock(PsiClass.class);
String superClassQualifiedName = "com.azure.data.appconfiguration.implementation.models"; // Extends java.lang.Object

int numOfInvocations = 1;
verifyRegisterProblem(classQualifiedName, interfaces, superClass, superClassQualifiedName, interfaceQualifiedName, numOfInvocations);
}

/**
* Test cases for the ImplementationTypeVisitor class.
* This case is for a non-Azure class.
* The registerProblem method should not be called.
*/
@Test
public void testUseOfNonAzurePackage() {
String classQualifiedName = "com.nonazure.data.appconfiguration.implementation.models";

// No interfaces implemented
PsiClass[] interfaces = new PsiClass[]{};
String interfaceQualifiedName = null;

// Extends Object (implicitly)
PsiClass superClass = mock(PsiClass.class);
String superClassQualifiedName = "java.lang.Object"; // Extends java.lang.Object

int numOfInvocations = 0;
verifyRegisterProblem(classQualifiedName, interfaces, superClass, superClassQualifiedName, interfaceQualifiedName, numOfInvocations);
}

/**
* Helper method to create visitor.
*
* @return ImplementationTypeVisitor
*/
JavaElementVisitor createVisitor() {
return new ImplementationTypeVisitor(mockHolder);
}

/**
* Helper method to verify registerProblem method.
*
* @param classQualifiedName Qualified name of the class
* @param interfaces Array of interfaces implemented by the class
* @param superClass Super class of the class - This is the class that the class extends
* @param superClassQualifiedName Qualified name of the super class
* @param interfaceQualifiedName Qualified name of the interface - This is the interface that the class implements
* @param numOfInvocations Number of times registerProblem method is called
*/
private void verifyRegisterProblem(String classQualifiedName, PsiClass[] interfaces, PsiClass superClass, String superClassQualifiedName, String interfaceQualifiedName, int numOfInvocations) {

PsiClassType type = mock(PsiClassType.class);
PsiClass psiClass = mock(PsiClass.class);
PsiIdentifier mockIdentifier = mock(PsiIdentifier.class);

when(mockVariable.getType()).thenReturn(type);

// isImplementationType method
when(type.resolve()).thenReturn(psiClass);
when(psiClass.getQualifiedName()).thenReturn(classQualifiedName);

// ExtendsOrImplementsImplementationType method
when(psiClass.getInterfaces()).thenReturn(interfaces);
when(psiClass.getSuperClass()).thenReturn(superClass);

when(superClass.getQualifiedName()).thenReturn(superClassQualifiedName);

if (interfaces.length > 0) {
when(interfaces[0].getQualifiedName()).thenReturn(interfaceQualifiedName);
}

when(mockVariable.getNameIdentifier()).thenReturn(mockIdentifier);
mockVisitor.visitVariable(mockVariable);
verify(mockHolder, times(numOfInvocations)).registerProblem(eq(mockIdentifier), Mockito.contains("Detected usage of an implementation type. Implementation types are not intended for public use. Use the publicly available Azure classes instead."));
}
}