-
Notifications
You must be signed in to change notification settings - Fork 177
Refactor Java Code Quality Analyzer #9654
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
Merged
+3,865
−12
Merged
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
d58f560
Initial code analyzer refactor
samvaity 18d4b1e
adding the resources directory
samvaity da3dde5
add changes for MavenUtil
samvaity 5bef2d9
apply feedback changes
samvaity c5e1757
readme updates
samvaity e275a81
move ruleconfig loading to startup activity + testing changes
samvaity 595e161
remove RuleConfigLoaderInitializer + comments
samvaity File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
74 changes: 74 additions & 0 deletions
74
...AndFeatures/azure-toolkit-for-intellij/azure-intellij-plugin-java-sdk/readme.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| # **Azure Toolkit for IntelliJ: Java SDK Integration** | ||
|
|
||
| The **Azure Toolkit for IntelliJ** is a project designed to empower Java developers by simplifying the creation, configuration, and usage of Azure services directly within IntelliJ IDEA. This plugin enhances productivity by providing seamless access to Azure SDKs and integrates a **Code Quality Analyzer Tool Window** that offers continuous analysis, real-time code suggestions, to improve Java code quality. | ||
|
|
||
| ## **Features** | ||
| - **Imported Rule Sets**: The plugin integrates with Azure SDKs to provide real-time code suggestions and best practices. | ||
| - **Code Quality Analyzer**: The tool window offers continuous analysis and recommendations to improve Java code quality. | ||
|
|
||
| ## **Integrated Rule Sets** | ||
|
|
||
| ### **1. Storage Upload without Length Check** | ||
| - **Anti-pattern**: Using Azure Storage upload APIs without a length parameter, causing the entire data payload to buffer in memory. | ||
| - **Issue**: Risks `OutOfMemoryErrors` for large files or high-volume uploads. | ||
| - **Severity**: INFO | ||
| - **Recommendation**: Use APIs that accept a length parameter. Refer to the [Azure SDK for Java documentation](https://learn.microsoft.com/en-us/azure/storage/blobs/storage-blob-upload-java) for details. | ||
|
|
||
| ### **2. Prefer ServiceBusProcessorClient over ServiceBusReceiverAsyncClient** | ||
| - **Anti-pattern**: Using the low-level `ServiceBusReceiverAsyncClient` API, which requires advanced Reactive programming skills. | ||
| - **Issue**: Increased complexity and potential misuse by non-experts in Reactive paradigms. | ||
| - **Severity**: WARNING | ||
| - **Recommendation**: Use the higher-level `ServiceBusProcessorClient` for simplified and efficient message handling. | ||
| [Learn more here](https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/servicebus/azure-messaging-servicebus/README.md#when-to-use-servicebusprocessorclient). | ||
|
|
||
| ### **3. Explicitly Disable Auto-Complete in ServiceBus Clients** | ||
| - **Anti-pattern**: Relying on default auto-completion without explicit verification. | ||
| - **Issue**: Messages may be incorrectly marked as completed even after processing failures. | ||
| - **Severity**: WARNING | ||
| - **Recommendation**: Use `disableAutoComplete()` to control message completion explicitly. See the [Azure ServiceBus documentation](https://learn.microsoft.com/en-us/java/api/com.azure.messaging.servicebus.servicebusclientbuilder.servicebusreceiverclientbuilder-disableautocomplete) for guidance. | ||
|
|
||
| ### **4. Avoid Dynamic Client Creation** | ||
| - **Anti-pattern**: Creating new client instances for each operation instead of reusing them. | ||
| - **Issue**: Leads to resource overhead, reduced performance, and increased latency. | ||
| - **Severity**: WARNING | ||
| - **Recommendation**: Reuse client instances throughout the application's lifecycle. | ||
| [Detailed guidance here](https://learn.microsoft.com/en-us/azure/developer/java/sdk/overview#connect-to-and-use-azure-resources-with-client-libraries). | ||
|
|
||
| ### **5. Avoid Hardcoded API Keys and Tokens** | ||
| - **Anti-pattern**: Storing sensitive credentials in source code. | ||
| - **Issue**: Exposes credentials to security breaches. | ||
| - **Severity**: WARNING | ||
| - **Recommendation**: Use `DefaultAzureCredential` or `AzureKeyCredential` for authentication. | ||
| [Learn more](https://learn.microsoft.com/en-us/java/api/com.azure.identity.defaultazurecredential?view=azure-java-stable). | ||
|
|
||
| ### **6. Use SyncPoller Instead of PollerFlux#getSyncPoller()** | ||
| - **Anti-pattern**: Converting asynchronous polling to synchronous with `getSyncPoller()`. | ||
| - **Issue**: Adds unnecessary complexity. | ||
| - **Severity**: WARNING | ||
| - **Recommendation**: Use `SyncPoller` directly for synchronous operations. | ||
| [Documentation link](https://learn.microsoft.com/java/api/com.azure.core.util.polling.syncpoller?view=azure-java-stable). | ||
|
|
||
| ### **7. Optimize Receive Mode and Prefetch Value** | ||
samvaity marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| - **Anti-pattern**: Using `PEEK_LOCK` with a high prefetch value. | ||
| - **Issue**: Can lead to performance bottlenecks and message lock expirations. | ||
| - **Severity**: WARNING | ||
| - **Recommendation**: Balance the prefetch value for efficient and concurrent processing. | ||
| [Learn more](https://learn.microsoft.com/en-us/azure/service-bus-messaging/service-bus-prefetch?tabs=dotnet#why-is-prefetch-not-the-default-option). | ||
samvaity marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| ### **8. Recommended Alternatives for Common APIs** | ||
|
|
||
| #### a. **Authentication**: Use `DefaultAzureCredential` over connection strings. | ||
| #### b. **Azure OpenAI**: Prefer `getChatCompletions` for conversational AI instead of `getCompletions`. | ||
| [More information here](https://learn.microsoft.com/java/api/overview/azure/ai-openai-readme?view=azure-java-preview). | ||
|
|
||
| ### **9. Batch Operations Instead of Single Operations in Loops** | ||
| - **Anti-pattern**: Performing repetitive single operations instead of batch processing. | ||
| - **Issue**: Inefficient resource use and slower execution. | ||
| - **Severity**: WARNING | ||
| - **Recommendation**: Utilize batch APIs for optimized resource usage. | ||
|
|
||
| ### **10. Use EventProcessorClient for Checkpoint Management** | ||
| - **Anti-pattern**: Calling `updateCheckpointAsync()` without proper blocking (`block()`). | ||
| - **Issue**: Results in ineffective checkpoint updates. | ||
| - **Severity**: WARNING | ||
| - **Recommendation**: Ensure the `block()` operator is used with an appropriate timeout for reliable checkpoint updates. | ||
85 changes: 85 additions & 0 deletions
85
...a/com/microsoft/azure/toolkit/intellij/java/sdk/analyzer/AbstractLibraryVersionCheck.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.microsoft.azure.toolkit.intellij.java.sdk.analyzer; | ||
|
|
||
| import com.intellij.codeInspection.LocalInspectionTool; | ||
| import com.intellij.codeInspection.ProblemsHolder; | ||
| import com.intellij.lang.StdLanguages; | ||
| import com.intellij.psi.PsiElement; | ||
| import com.intellij.psi.xml.XmlFile; | ||
| import com.intellij.psi.xml.XmlTag; | ||
| import com.microsoft.azure.toolkit.intellij.java.sdk.models.RuleConfig; | ||
| import java.io.IOException; | ||
| import org.jetbrains.idea.maven.project.MavenProjectsManager; | ||
|
|
||
| /** | ||
| * Abstract class for the library version check inspection. The UpgradeLibraryVersionCheck and | ||
| * IncompatibleDependencyCheck classes extend this class. | ||
| * <p> | ||
| * The UpgradeLibraryVersionCheck class checks the version of the libraries in the pom.xml file against the recommended | ||
| * version. The IncompatibleDependencyCheck class checks the version of the libraries in the pom.xml file against | ||
| * compatible versions. | ||
| */ | ||
| public abstract class AbstractLibraryVersionCheck extends LocalInspectionTool { | ||
samvaity marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Method to check the pom.xml file for the libraries and their versions. | ||
| * | ||
| * @param file The pom.xml file to check for the libraries and their versions | ||
| * @param holder The holder for the problems found in the file | ||
| * | ||
| * @throws IOException If an error occurs while reading the file | ||
| */ | ||
| protected void checkPomXml(XmlFile file, ProblemsHolder holder) throws IOException { | ||
| MavenProjectsManager mavenProjectsManager = MavenProjectsManager.getInstance(file.getProject()); | ||
| if (!mavenProjectsManager.isMavenizedProject()) { | ||
| return; | ||
| } | ||
|
|
||
| XmlFile xmlFile = (XmlFile) file.getViewProvider().getPsi(StdLanguages.XML); | ||
| XmlTag rootTag = xmlFile.getRootTag(); | ||
|
|
||
| if (rootTag != null && "project".equals(rootTag.getName())) { | ||
| for (XmlTag dependenciesTag : rootTag.findSubTags("dependencies")) { | ||
| for (XmlTag dependencyTag : dependenciesTag.findSubTags("dependency")) { | ||
| XmlTag groupIdTag = dependencyTag.findFirstSubTag("groupId"); | ||
| XmlTag artifactIdTag = dependencyTag.findFirstSubTag("artifactId"); | ||
| XmlTag versionTag = dependencyTag.findFirstSubTag("version"); | ||
|
|
||
| if (groupIdTag != null && artifactIdTag != null && versionTag != null) { | ||
| String fullName = groupIdTag.getValue().getText() + ":" + artifactIdTag.getValue().getText(); | ||
| String currentVersion = versionTag.getValue().getText(); | ||
| this.checkAndFlagVersion(fullName, currentVersion, holder, versionTag); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Method to get the formatted message for the anti-pattern. | ||
| * | ||
| * @param fullName The full name of the library eg "com.azure:azure-core" | ||
| * @param recommendedVersion The recommended version of the library eg "1.0" | ||
| * @param RULE_CONFIG The rule configuration object | ||
| * | ||
| * @return The formatted message for the anti-pattern with the full name and recommended version | ||
| */ | ||
| protected static String getFormattedMessage(String fullName, String recommendedVersion, RuleConfig RULE_CONFIG) { | ||
| return RULE_CONFIG.getAntiPatternMessage() | ||
| .replace("{{fullName}}", fullName) | ||
| .replace("{{recommendedVersion}}", recommendedVersion); | ||
| } | ||
|
|
||
| /** | ||
| * Abstract method to check the version of the library and flag it if necessary. | ||
| * | ||
| * @param fullName The full name of the library eg "com.azure:azure-core" | ||
| * @param currentVersion The current version of the library eg "1.0" | ||
| * @param holder The holder for the problems found | ||
| * @param versionElement The element for the version of the library | ||
| */ | ||
| protected abstract void checkAndFlagVersion(String fullName, String currentVersion, ProblemsHolder holder, | ||
| PsiElement versionElement) throws IOException; | ||
| } | ||
92 changes: 92 additions & 0 deletions
92
...in/java/com/microsoft/azure/toolkit/intellij/java/sdk/analyzer/AsyncSubscribeChecker.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.microsoft.azure.toolkit.intellij.java.sdk.analyzer; | ||
|
|
||
| import com.intellij.codeInspection.LocalInspectionTool; | ||
| import com.intellij.psi.PsiClass; | ||
| import com.intellij.psi.PsiClassType; | ||
| import com.intellij.psi.PsiElement; | ||
| import com.intellij.psi.PsiExpression; | ||
| import com.intellij.psi.PsiMethodCallExpression; | ||
| import com.intellij.psi.PsiParameter; | ||
| import com.intellij.psi.PsiReferenceExpression; | ||
| import com.intellij.psi.PsiType; | ||
| import com.microsoft.azure.toolkit.intellij.java.sdk.models.RuleConfig; | ||
| import java.util.List; | ||
|
|
||
| /** | ||
| * Abstract base class for checking the usage of the following method to be ''subscribe'' in provided context. | ||
| */ | ||
| public abstract class AsyncSubscribeChecker extends LocalInspectionTool { | ||
| /** | ||
| * Checks if the method call is following a specific method call like `subscribe`. | ||
| * | ||
| * @param expression The method call expression to analyze. | ||
| * | ||
| * @return True if the method call is following a `subscribe` method call, false otherwise. | ||
| */ | ||
| protected static boolean isFollowingMethodSubscribe(PsiMethodCallExpression expression) { | ||
samvaity marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| // Check if the parent element is a method call expression | ||
| if (!(expression.getParent() instanceof PsiReferenceExpression reference)) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if the grandparent element is a method call expression | ||
| PsiElement grandParent = reference.getParent(); | ||
| if (!(grandParent instanceof PsiMethodCallExpression parentCall)) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if the parent method call's name is "subscribe" | ||
| return "subscribe".equals(parentCall.getMethodExpression().getReferenceName()); | ||
| } | ||
|
|
||
| /** | ||
| * This method checks if the method call is made on an object from the provided Azure SDK context. | ||
| * | ||
| * @param expression The method call expression to analyze. | ||
| * @param scopeToCheck The list of class contexts to check. | ||
| * | ||
| * @return True if the method call is made on an object in the provided Azure SDK context, false otherwise. | ||
| */ | ||
| protected static boolean isCalledInProvidedContext(PsiMethodCallExpression expression, List<String> scopeToCheck) { | ||
| // Get the qualifier expression from the method call expression | ||
| PsiExpression qualifier = expression.getMethodExpression().getQualifierExpression(); | ||
| if (!(qualifier instanceof PsiReferenceExpression)) { | ||
| return false; | ||
| } | ||
|
|
||
| // Resolve the qualifier element | ||
| PsiElement resolvedElement = ((PsiReferenceExpression) qualifier).resolve(); | ||
| if (!(resolvedElement instanceof PsiParameter)) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check the parameter type | ||
| PsiParameter parameter = (PsiParameter) resolvedElement; | ||
| PsiType parameterType = parameter.getType(); | ||
|
|
||
| // Verify if the parameter type is within the provided context | ||
| if (!(parameterType instanceof PsiClassType)) { | ||
| return false; | ||
| } | ||
|
|
||
| PsiClassType classType = (PsiClassType) parameterType; | ||
| PsiClass psiClass = classType.resolve(); | ||
| if (psiClass == null) { | ||
| return false; | ||
| } | ||
|
|
||
| String qualifiedName = psiClass.getQualifiedName(); | ||
| if (qualifiedName == null) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if the qualified name matches the Azure SDK context | ||
| boolean isInProvidedScope = scopeToCheck.contains(parameterType.getCanonicalText()); | ||
| boolean isAzureSDKContext = qualifiedName.startsWith(RuleConfig.AZURE_PACKAGE_NAME); | ||
|
|
||
| return isInProvidedScope && isAzureSDKContext; | ||
| } | ||
| } | ||
18 changes: 18 additions & 0 deletions
18
...in/java/com/microsoft/azure/toolkit/intellij/java/sdk/analyzer/ConnectionStringCheck.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.microsoft.azure.toolkit.intellij.java.sdk.analyzer; | ||
|
|
||
| import com.microsoft.azure.toolkit.intellij.java.sdk.models.RuleConfig; | ||
| import com.microsoft.azure.toolkit.intellij.java.sdk.utils.RuleConfigLoader; | ||
|
|
||
| /** | ||
| * Inspection tool to check discouraged Connection String usage. | ||
| */ | ||
| public class ConnectionStringCheck extends DetectDiscouragedAPIUsageCheck { | ||
|
|
||
| @Override | ||
| protected RuleConfig getRuleConfig() { | ||
| return RuleConfigLoader.getInstance().getRuleConfig("ConnectionStringCheck"); | ||
| } | ||
| } |
105 changes: 105 additions & 0 deletions
105
...om/microsoft/azure/toolkit/intellij/java/sdk/analyzer/DetectDiscouragedAPIUsageCheck.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.microsoft.azure.toolkit.intellij.java.sdk.analyzer; | ||
|
|
||
| import com.intellij.codeInspection.LocalInspectionTool; | ||
| import com.intellij.codeInspection.ProblemsHolder; | ||
| import com.intellij.psi.JavaElementVisitor; | ||
| import com.intellij.psi.PsiClass; | ||
| import com.intellij.psi.PsiElement; | ||
| import com.intellij.psi.PsiElementVisitor; | ||
| import com.intellij.psi.PsiMethod; | ||
| import com.intellij.psi.PsiMethodCallExpression; | ||
| import com.intellij.psi.PsiReferenceExpression; | ||
| import com.microsoft.azure.toolkit.intellij.java.sdk.models.RuleConfig; | ||
| import java.util.HashSet; | ||
| import java.util.Set; | ||
| import org.jetbrains.annotations.NotNull; | ||
|
|
||
| /** | ||
| * Abstract base class to check for the use of discouraged APIs in the code. | ||
| */ | ||
| public abstract class DetectDiscouragedAPIUsageCheck extends LocalInspectionTool { | ||
|
|
||
| /** | ||
| * Provides the specific RuleConfig for the subclass context. | ||
| * | ||
| * @return RuleConfig for the subclass | ||
| */ | ||
| protected abstract RuleConfig getRuleConfig(); | ||
|
|
||
| @NotNull | ||
| @Override | ||
| public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { | ||
| RuleConfig ruleConfig = getRuleConfig(); | ||
|
|
||
| if (ruleConfig.skipRuleCheck()) { | ||
| return PsiElementVisitor.EMPTY_VISITOR; | ||
| } | ||
|
|
||
| return new DetectDiscouragedAPIUsageVisitor(holder, ruleConfig); | ||
| } | ||
|
|
||
| static class DetectDiscouragedAPIUsageVisitor extends JavaElementVisitor { | ||
samvaity marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| private final ProblemsHolder holder; | ||
| private final RuleConfig ruleConfig; | ||
|
|
||
| DetectDiscouragedAPIUsageVisitor(ProblemsHolder holder, RuleConfig ruleConfig) { | ||
| this.holder = holder; | ||
| this.ruleConfig = ruleConfig; | ||
| } | ||
|
|
||
| @Override | ||
| public void visitElement(@NotNull PsiElement element) { | ||
| super.visitElement(element); | ||
|
|
||
| // Ensure the element is a method call | ||
| if (!(element instanceof PsiMethodCallExpression methodCallExpression)) { | ||
| return; | ||
| } | ||
|
|
||
| // Resolve the method being called | ||
| PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); | ||
| PsiElement resolvedMethod = methodExpression.resolve(); | ||
| if (!(resolvedMethod instanceof PsiMethod method)) { | ||
| return; | ||
| } | ||
|
|
||
| // Get the containing class of the method | ||
| PsiClass containingClass = method.getContainingClass(); | ||
| if (containingClass == null) { | ||
| return; | ||
| } | ||
|
|
||
| // Get qualified name of the containing class | ||
| String classQualifiedName = containingClass.getQualifiedName(); | ||
| if (classQualifiedName == null) { | ||
| return; | ||
| } | ||
|
|
||
| // Check if the method is a discouraged API | ||
| String methodName = method.getName(); | ||
| if (!ruleConfig.getUsagesToCheck().contains(methodName)) { | ||
| return; | ||
| } | ||
|
|
||
| // Verify package name and scope | ||
| if (!classQualifiedName.startsWith(RuleConfig.AZURE_PACKAGE_NAME)) { | ||
| return; | ||
| } | ||
|
|
||
| Set<String> scopesToCheck = new HashSet<>(ruleConfig.getScopeToCheck()); | ||
samvaity marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (scopesToCheck.stream().noneMatch(classQualifiedName::startsWith)) { | ||
| return; | ||
| } | ||
|
|
||
| // Register a problem for the discouraged API usage | ||
| PsiElement problemElement = methodExpression.getReferenceNameElement(); | ||
| if (problemElement != null) { | ||
| holder.registerProblem(problemElement, ruleConfig.getAntiPatternMessage()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.