From a828a349680805911c8bc49675bfdd52acb13d85 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 2 Aug 2023 15:11:41 +0200 Subject: [PATCH] [java-highlighting] Fixes in unnamed variables highlighting (IDEA-323960) 1. Highlight C-style arrays 2. Do not highlight variables inside for initializer 3. Highlight variables without initializer 4. Better message for underscore references when unnamed variables are allowed GitOrigin-RevId: 5bca18969cf8fb0ea6e052b0aef71323bbfa69b5 --- .../messages/JavaAnalysisBundle.properties | 5 +-- .../daemon/impl/analysis/HighlightUtil.java | 36 +++++++++++++++---- .../impl/analysis/HighlightVisitorImpl.java | 2 +- .../src/messages/JavaErrorBundle.properties | 1 + .../UnnamedPatterns.java | 2 +- .../UnnamedVariables.java | 33 ++++++++++++----- 6 files changed, 59 insertions(+), 20 deletions(-) diff --git a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties index 26516c619369..a80bf1b5af74 100644 --- a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties +++ b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties @@ -650,7 +650,8 @@ notification.file.system.issue=File Operation Issue notification.content.cannot.move.file=Cannot move ''{0}'' into ''{1}'': {2} intention.family.name.replace.with.unnamed.pattern=Replace with unnamed pattern intention.name.ignore.exception=Ignore exception ''{0}'' -error.unnamed.variable.not.allowed=Unnamed variable is not allowed error.unnamed.field.not.allowed=Unnamed field is not allowed error.unnamed.method.parameter.not.allowed=Unnamed method parameter is not allowed -error.unnamed.local.variable.not.allowed.in.this.context=Unnamed local variable is not allowed in this context +error.unnamed.variable.not.allowed.in.this.context=Unnamed variable declaration is not allowed in this context +error.unnamed.variable.brackets=Brackets are not allowed after unnamed variable declaration +error.unnamed.variable.without.initializer=Unnamed variable declaration must have an initializer diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java index a4d3156c9232..faf4a7e43685 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java @@ -68,6 +68,7 @@ import com.siyeh.ig.psiutils.ControlFlowUtils; import com.siyeh.ig.psiutils.InstanceOfUtils; import com.siyeh.ig.psiutils.VariableAccessUtils; import com.siyeh.ig.psiutils.VariableNameGenerator; +import one.util.streamex.StreamEx; import org.jetbrains.annotations.*; import java.awt.*; @@ -840,7 +841,9 @@ public final class HighlightUtil { PsiElement parent = identifier.getParent(); if (languageLevel.isAtLeast(LanguageLevel.JDK_1_9) && !(parent instanceof PsiUnnamedPattern) && !(parent instanceof PsiVariable var && var.isUnnamed())) { - String text = JavaErrorBundle.message("underscore.identifier.error"); + String text = HighlightingFeature.UNNAMED_PATTERNS_AND_VARIABLES.isSufficient(languageLevel) ? + JavaErrorBundle.message("underscore.identifier.error.unnamed") + : JavaErrorBundle.message("underscore.identifier.error"); return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(identifier).descriptionAndTooltip(text); } else if (languageLevel.isAtLeast(LanguageLevel.JDK_1_8)) { @@ -855,13 +858,27 @@ public final class HighlightUtil { return null; } - static HighlightInfo.Builder checkAllowedUnnamedLocation(@NotNull PsiVariable variable) { + static HighlightInfo.Builder checkUnnamedVariableDeclaration(@NotNull PsiVariable variable) { + if (isArrayDeclaration(variable)) { + IntentionAction fix = new NormalizeBracketsFix(variable).asIntention(); + TokenSet brackets = TokenSet.create(JavaTokenType.LBRACKET, JavaTokenType.RBRACKET); + TextRange range = StreamEx.of(variable.getChildren()) + .filter(t -> PsiUtil.isJavaToken(t, brackets)) + .map(PsiElement::getTextRangeInParent) + .reduce(TextRange::union) + .orElseThrow() + .shiftRight(variable.getTextRange().getStartOffset());// Must have at least one + return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(range).descriptionAndTooltip( + JavaAnalysisBundle.message("error.unnamed.variable.brackets")).registerFix(fix, null, null, null, null); + } if (variable instanceof PsiPatternVariable) return null; if (variable instanceof PsiResourceVariable) return null; String message; + IntentionAction fix = null; if (variable instanceof PsiLocalVariable local) { - if (local.getParent() instanceof PsiDeclarationStatement decl && decl.getParent() instanceof PsiCodeBlock) return null; - message = JavaAnalysisBundle.message("error.unnamed.local.variable.not.allowed.in.this.context"); + if (local.getInitializer() != null) return null; + message = JavaAnalysisBundle.message("error.unnamed.variable.without.initializer"); + fix = getFixFactory().createAddVariableInitializerFix(local); } else if (variable instanceof PsiParameter parameter) { PsiElement scope = parameter.getDeclarationScope(); @@ -872,10 +889,15 @@ public final class HighlightUtil { message = JavaAnalysisBundle.message("error.unnamed.field.not.allowed"); } else { - message = JavaAnalysisBundle.message("error.unnamed.variable.not.allowed"); + message = JavaAnalysisBundle.message("error.unnamed.variable.not.allowed.in.this.context"); } - return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(Objects.requireNonNull(variable.getNameIdentifier())) - .descriptionAndTooltip(message); + TextRange range = TextRange.create(variable.getTextRange().getStartOffset(), + Objects.requireNonNull(variable.getNameIdentifier()).getTextRange().getEndOffset()); + HighlightInfo.Builder builder = HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(range).descriptionAndTooltip(message); + if (fix != null) { + builder.registerFix(fix, null, null, null, null); + } + return builder; } @NotNull diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java index 35eaab04245e..56d5f4e1d9f3 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java @@ -780,7 +780,7 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh if (notAvailable != null) { add(notAvailable); } else { - add(HighlightUtil.checkAllowedUnnamedLocation(variable)); + add(HighlightUtil.checkUnnamedVariableDeclaration(variable)); } } diff --git a/java/java-psi-impl/src/messages/JavaErrorBundle.properties b/java/java-psi-impl/src/messages/JavaErrorBundle.properties index 8214e1b73a83..922a9f47448e 100644 --- a/java/java-psi-impl/src/messages/JavaErrorBundle.properties +++ b/java/java-psi-impl/src/messages/JavaErrorBundle.properties @@ -416,6 +416,7 @@ override.not.allowed.in.interfaces=@Override is not allowed when implementing in declaration.not.allowed=Declaration not allowed here underscore.identifier.error=Since Java 9, '_' is a keyword, and may not be used as an identifier +underscore.identifier.error.unnamed=Using '_' as a reference is not allowed underscore.lambda.identifier=Use of '_' as a lambda parameter name is not allowed assert.identifier.warn=Use of 'assert' as an identifier is not supported in releases since Java 1.4 diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatterns/UnnamedPatterns.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatterns/UnnamedPatterns.java index 9a930f23aff1..1c3c17e07c04 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatterns/UnnamedPatterns.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatterns/UnnamedPatterns.java @@ -2,7 +2,7 @@ public class UnnamedPatterns { record R(int a, int b) {} void test(Object obj) { - if (obj instanceof _) {} + if (obj instanceof _) {} if (obj instanceof R(_, _)) {} if (obj instanceof R(int a, _)) { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingUnnamed/UnnamedVariables.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingUnnamed/UnnamedVariables.java index 6f973d31a4bc..bb9e38681586 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingUnnamed/UnnamedVariables.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingUnnamed/UnnamedVariables.java @@ -1,28 +1,43 @@ import java.util.function.*; public class UnnamedVariables { - void testParameter(int _, String _) { - System.out.println(_); + void testParameter(int _, String _) { + System.out.println(_); } - int _ = 123; - String s = _; + int _ = 123; + String s = _; void testLambda() { Consumer consumer = _ -> System.out.println("Hello"); - Consumer consumer2 = _ -> System.out.println(_); - Consumer consumer3 = _ -> System.out.println(_.trim()); + Consumer consumer2 = _ -> System.out.println(_); + Consumer consumer3 = _ -> System.out.println(_.trim()); Consumer consumer4 = _ -> { - var v = _; - System.out.println(v.trim()); + var v = _; + System.out.println(v.trim()); }; BiConsumer consumer5 = (_,_) -> {}; } + void testWhen(Object obj) { + switch (obj) { + case String _ when _.isEmpty() -> {} + } + } + void testLocal() { int _ = 10; int _ = 20; - for (int _ = 1;;) {} + int _[] = {30}; + int[] _ = {40}; + var _ = "string"; + for (int _ = 1;;) {} + } + + void testNoInitializer() { + int _; + + for(int _;;) {} } void testCatch() {