Skip to content

Conversation

@FxMorin
Copy link
Contributor

@FxMorin FxMorin commented Aug 14, 2024

Description:

Currently some inspections don't support strings that have been split to support a line width limit, same goes for Injection method targets.

Changes:

Removed multiple direct PsiLiteral checks which where not needed, since all calls would lead to PsiElement.constantStringValue, which will return null if its not suppose to.

Now, all these calls will use the ConstantExpressionVisitor.compute call which takes care of the split string logic.

Examples:

Method Reference

Before - Single (Expected behaviour)
Before - Single
Before - Split
Before - Split

After - Split
After - Split

AmbiguousReferenceInspection

Before
Before

After
After

InvalidMemberReferenceInspection

Before
Before

After
After

UnnecessaryQualifiedMemberReferenceInspection

Before
Before

After
After

Copy link
Member

@Earthcomputer Earthcomputer left a comment

Choose a reason for hiding this comment

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

Are you sure that checks for PsiLiteral weren't the only problem? The constantEvaluationHelper is already supposed to handle string concatenation, see ConstantExpressionVisitor.compute

@FxMorin
Copy link
Contributor Author

FxMorin commented Aug 14, 2024

Are you sure that checks for PsiLiteral weren't the only problem? The constantEvaluationHelper is already supposed to handle string concatenation, see ConstantExpressionVisitor.compute

I was not aware of this. I just tested it, and you're right. I'll remove the changes to PsiElement.constantStringValue

@FxMorin FxMorin force-pushed the feature/split-string-constants branch from c2204df to 8b41280 Compare August 14, 2024 07:11
@FxMorin FxMorin requested a review from Earthcomputer August 14, 2024 07:11
@Earthcomputer Earthcomputer merged commit 8e06246 into minecraft-dev:dev Aug 14, 2024
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.

2 participants