checking for use of implementation types #24
Conversation
...n/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/ImplementationTypeCheck.java
Outdated
Show resolved
Hide resolved
| return false; | ||
| } | ||
| // 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(RULE_CONFIG.getListedItemsToCheck().get(0)); |
There was a problem hiding this comment.
We should not use get(0) as this will only check the first element in the list of configured rules.
There was a problem hiding this comment.
There's only one entry and its in String form.
We ended up with .get(0) for cases where there's only one String listedItemToCheck, but other rules have a list listedItemToCheck so instead of having a RULE_CONFIG attribute for String ItemToCheck and List listedItemToCheck we can just keep the list attribute, and when a String is encountered, it returns a list with one entry
...re-intellij-plugin-azure-sdk/src/main/resources/META-INF/azure-intellij-plugin-azure-sdk.xml
Outdated
Show resolved
Hide resolved
| "antiPatternMessage": "Endpoint should not be used with KeyCredential for non-Azure OpenAI clients" | ||
| }, | ||
| "ImplementationTypeCheck": { | ||
| "typesToCheck": "implementation", |
There was a problem hiding this comment.
This is not really a "type". It's a sub-package name. So, we should consider changing the name here.
There was a problem hiding this comment.
done - using subPackageName now
| if (interfaces.length == 0 && superClass != null && superClass.getQualifiedName().equals("java.lang.Object")) { | ||
| return false; | ||
| } | ||
|
|
||
| // Case 2: Class has implemented interfaces or extends a class that is an implementation type | ||
| if (interfaces.length > 0 || (superClass != null && !superClass.getQualifiedName().startsWith("java."))) { |
There was a problem hiding this comment.
Why do we need to check this? We should check if any of the parent classes are using Azure types and not check for super types that are in java.* package.
There was a problem hiding this comment.
this was eliminating types in the java.* package so to check for everything else.
I've refactored the whole extendsOrImplementsImplementationType method to check for chains of superclasses & interfaces now
| } | ||
|
|
||
| // If the super class is from the Azure package and is an implementation type return true | ||
| String superClassName = superClass.getQualifiedName(); |
There was a problem hiding this comment.
There can be a chain of superclasses. We should check for the whole chain.
There was a problem hiding this comment.
I've refactored the whole extendsOrImplementsImplementationType method to check for chains of superclasses & interfaces now
...n/java/com/microsoft/azure/toolkit/intellij/azure/sdk/buildtool/ImplementationTypeCheck.java
Outdated
Show resolved
Hide resolved
|
|
||
| // If the super class is from the Azure package and is an implementation type return true | ||
| String superClassName = superClass.getQualifiedName(); | ||
| return superClassName != null && superClassName.startsWith(RuleConfig.AZURE_PACKAGE_NAME) && superClassName.contains(RULE_CONFIG.getListedItemsToCheck().get(0)); |
There was a problem hiding this comment.
We should call the isImplementationType method instead of duplicating the logic.
There was a problem hiding this comment.
I've refactored the whole extendsOrImplementsImplementationType method to recursively check for chains of superclasses & interfaces now
What does this implement/fix? Explain your changes.
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.
Has this been tested?
Any relevant logs, screenshots, error output, etc.?