diff --git a/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/README.md b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/README.md index 78f2c3415fe..c1ee071c753 100644 --- a/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/README.md +++ b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/README.md @@ -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. diff --git a/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/ImplementationTypeCheck.java b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/ImplementationTypeCheck.java new file mode 100644 index 00000000000..9b9a5fa5b48 --- /dev/null +++ b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/ImplementationTypeCheck.java @@ -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 myList = new ArrayList<>(); -> type = List + 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; + } + } +} diff --git a/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/RuleConfigLoader.java b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/RuleConfigLoader.java index 09c60719aef..1bf7916203a 100644 --- a/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/RuleConfigLoader.java +++ b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/RuleConfigLoader.java @@ -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 diff --git a/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/resources/META-INF/azure-intellij-plugin-azure-sdk.xml b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/resources/META-INF/azure-intellij-plugin-azure-sdk.xml index 14668726b33..296ae0b74f3 100644 --- a/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/resources/META-INF/azure-intellij-plugin-azure-sdk.xml +++ b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/resources/META-INF/azure-intellij-plugin-azure-sdk.xml @@ -261,5 +261,19 @@ com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool.IncompatibleDependencyCheck + + + + + com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool.ImplementationTypeCheck + + diff --git a/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/resources/META-INF/ruleConfigs.json b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/resources/META-INF/ruleConfigs.json index ed53fc1cb91..a69b13c23a3 100644 --- a/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/resources/META-INF/ruleConfigs.json +++ b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/main/resources/META-INF/ruleConfigs.json @@ -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." } } diff --git a/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/test/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/ImplementationTypeCheckTest.java b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/test/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/ImplementationTypeCheckTest.java new file mode 100644 index 00000000000..e87dc2205ac --- /dev/null +++ b/PluginsAndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-azure-sdk/src/test/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/ImplementationTypeCheckTest.java @@ -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.")); + } +}