SONARPY-2321 Fix quality gate issues #2145
Conversation
hashicorp-vault-sonar-prod
bot
changed the title
thomas-serre-sonarsource
changed the title
|
maksim-grebeniuk-sonarsource
requested a review
from guillaume-dequenne-sonarsource
thomas-serre-sonarsource
changed the title
| @@ -43,12 +43,12 @@ public class FunctionDescriptor implements Descriptor { | |||
| private final TypeAnnotationDescriptor typeAnnotationDescriptor; | |||
|
|
|||
|
|
|||
| public FunctionDescriptor(String name, String fullyQualifiedName, List<Parameter> parameters, boolean isAsynchronous, | |||
| public FunctionDescriptor(String name, @Nullable String fullyQualifiedName, List<Parameter> parameters, boolean isAsynchronous, | |||
There was a problem hiding this comment.
After second look, I think fully qualified names for FunctionDescriptor should never be null again.
In DescriptorsToProtobuf, we're making null checks during deserialization from the cache because it used to be nullable (thus serialized with an empty value).
Due to our cacheVersion checks, we'd disregard any cached value that is null, so we should be safe to remove this check.
Note that if we actually do that:
- It breaks
ClassDescriptorTestwhich are based on a V1->Descriptor converison. - It is no longer consistent with the
descriptors.protomodel that says thatFunctionDescriptor#fullyQualifiedNameis an optional field.
Fixing this would significantly expand the scope of this PR, so I'd suggest creating a hardening ticket to ensure FunctionDescriptor#fullyQualifiedName is not longer nullable and adapt the related tests there, wdyt?
There was a problem hiding this comment.
As discussed live, we leave the current PR as is and create two tickets:
- One to remove the nullability of
fullyqualifiedname(SONARPY-2335) - One to remove the conversion logic from V1 symbol to descriptor (SONARPY-2336)
guillaume-dequenne-sonarsource
deleted the
ts/fix-quality-gate-issues
branch
No description provided.