Skip to content

Conversation

samvaity
Copy link
Contributor

@samvaity samvaity commented Jan 6, 2025

What does this implement/fix? Explain your changes.

This PR includes changes for Azure Toolkit for IntelliJ's Code Quality Analyzer to integrate rule sets for flagging and making better Azure SDK usage.

Integrated Rule Sets:

  1. Storage:

    • Ensure Storage Upload APIs use a length parameter to prevent memory issues.
  2. Identity & Security:

    • Avoid hardcoded API keys; use DefaultAzureCredential instead.
  3. Asynchronous Programming:

    • Use SyncPoller instead of PollerFlux#getSyncPoller() for simplicity.
  4. Messaging (AMQP & Service Bus):

    • Prefer ServiceBusProcessorClient over ServiceBusReceiverAsyncClient.
    • Explicitly disable auto-complete in Service Bus clients to avoid message loss.
    • Optimize Receive Mode & Prefetch Value for efficient processing.
    • Use EventProcessorClient for checkpoint management.
  5. General Best Practices:

    • Batch operations instead of single operations in loops.
    • Use recommended authentication methods over connection strings.
    • For Azure OpenAI, prefer getChatCompletions over getCompletions.

Does this close any currently open issues?

Closes open Pull Request #8584

Any relevant logs, screenshots, error output, etc.?

  1. Storage Uppload API without length check:
Screenshot 2025-01-29 122944
  1. DisableAutoComplete check for ServiceBus
    image

  2. Prefer EventProcessorClient instead of EventHubConsumerAsyncClient

Screenshot 2025-01-29 163020
  1. Block over Async clients check
Screenshot 2025-01-29 132526
  1. Use of hardcoded keys in key based Authentication
Screenshot 2025-01-29 123029

Any other comments?

Has this been tested?

  • Tested

@samvaity samvaity changed the title Initial code analyzer refactor Java Code Quality Analyzer refactor Jan 6, 2025
@samvaity samvaity changed the title Java Code Quality Analyzer refactor Refactor Java Code Quality Analyzer Jan 6, 2025
@@ -31,7 +32,7 @@ public class DynamicClientCreationCheck extends LocalInspectionTool {
@NotNull
@Override
public JavaElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
return new DynamicClientCreationVisitor(holder, RuleConfigLoader.getInstance());
return new DynamicClientCreationVisitor(holder, RuleConfigLoader.getInstance().getRuleConfigs());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure, do you know if this could run prior to ProjectActivity? If that's the case, there might be a NPE, given that INSTANCE might not have initialized yet.

Copy link
Contributor Author

@samvaity samvaity Feb 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did do a runIde for testing this strategy. It does seem to be working as expected.
Also, from the documentation here, <postStartupActivity/> is executed after project initialization but before the first UI interaction. So, it should be safe relying on the project being fully initialized can safely execute logic.

Copy link
Collaborator

@wangmingliang-ms wangmingliang-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wangmingliang-ms wangmingliang-ms merged commit d87d4ce into microsoft:develop Feb 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants