From c2e1f6fae18b2dddb398cc32149e33571082bedd Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Thu, 5 Dec 2024 21:13:02 -0500 Subject: [PATCH] Fix Bug 96932 (#1840) - properly check context in an enum method before offering to create a new enum constant and possibly offer a new field or constant instead - add new tests to UnresolvedVariablesQuickFixTest - fixes #1839 --- .../UnresolvedElementsBaseSubProcessor.java | 116 +++++-- .../UnresolvedVariablesQuickFixTest.java | 320 +++++++++++++++++- 2 files changed, 402 insertions(+), 34 deletions(-) diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/UnresolvedElementsBaseSubProcessor.java b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/UnresolvedElementsBaseSubProcessor.java index 71c658c5275..0986013f783 100644 --- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/UnresolvedElementsBaseSubProcessor.java +++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/UnresolvedElementsBaseSubProcessor.java @@ -80,6 +80,8 @@ import org.eclipse.jdt.core.dom.ITypeBinding; import org.eclipse.jdt.core.dom.IVariableBinding; import org.eclipse.jdt.core.dom.ImportDeclaration; +import org.eclipse.jdt.core.dom.InfixExpression; +import org.eclipse.jdt.core.dom.InfixExpression.Operator; import org.eclipse.jdt.core.dom.LambdaExpression; import org.eclipse.jdt.core.dom.MemberValuePair; import org.eclipse.jdt.core.dom.MethodDeclaration; @@ -92,6 +94,7 @@ import org.eclipse.jdt.core.dom.ParenthesizedExpression; import org.eclipse.jdt.core.dom.ProvidesDirective; import org.eclipse.jdt.core.dom.QualifiedName; +import org.eclipse.jdt.core.dom.ReturnStatement; import org.eclipse.jdt.core.dom.SimpleName; import org.eclipse.jdt.core.dom.SimpleType; import org.eclipse.jdt.core.dom.SingleMemberAnnotation; @@ -211,6 +214,7 @@ protected UnresolvedElementsBaseSubProcessor() { private final String ADD_IMPORT_ID= "org.eclipse.jdt.ui.correction.addImport"; //$NON-NLS-1$ + @SuppressWarnings({ "deprecation" }) public void collectVariableProposals(IInvocationContext context, IProblemLocation problem, IVariableBinding resolvedField, Collection proposals) throws CoreException { ICompilationUnit cu= context.getCompilationUnit(); @@ -450,44 +454,90 @@ private void addNewFieldForType(ICompilationUnit targetCU, ITypeBinding binding, String name= simpleName.getIdentifier(); String nameLabel= BasicElementLabels.getJavaElementName(name); String label; + boolean createEnumProposal= false; if (senderDeclBinding.isEnum() && !isWriteAccess) { - label= Messages.format(CorrectionMessages.UnresolvedElementsSubProcessor_createenum_description, new Object[] { nameLabel, ASTResolving.getTypeSignature(senderDeclBinding) }); - NewVariableCorrectionProposalCore core= new NewVariableCorrectionProposalCore(label, targetCU, NewVariableCorrectionProposalCore.ENUM_CONST, simpleName, senderDeclBinding, 10, false); - T p1= newVariableCorrectionProposalToT(core, NewFieldForTypeProposal1); - if (p1 != null) - proposals.add(p1); - } else { - if (!mustBeConst) { - int fieldRelevance= StubUtility.hasFieldName(targetCU.getJavaProject(), name) ? IProposalRelevance.CREATE_FIELD_PREFIX_OR_SUFFIX_MATCH : IProposalRelevance.CREATE_FIELD; - int uid; - if (binding == null) { - label= Messages.format(CorrectionMessages.UnresolvedElementsSubProcessor_createfield_description, nameLabel); - uid= NewFieldForTypeProposal2; - } else { - label= Messages.format(CorrectionMessages.UnresolvedElementsSubProcessor_createfield_other_description, new Object[] { nameLabel, ASTResolving.getTypeSignature(senderDeclBinding) } ); - uid= NewFieldForTypeProposal3; + createEnumProposal= true; + if (simpleName.getParent() instanceof ReturnStatement retStmt) { + createEnumProposal= false; + MethodDeclaration methodDecl= ASTNodes.getFirstAncestorOrNull(retStmt, MethodDeclaration.class); + if (methodDecl != null) { + IMethodBinding methodBinding= methodDecl.resolveBinding(); + ITypeBinding retTypeBinding= methodBinding.getReturnType(); + if (senderDeclBinding.isEqualTo(retTypeBinding)) { + createEnumProposal= true; + } } - NewVariableCorrectionProposalCore core= new NewVariableCorrectionProposalCore(label, targetCU, NewVariableCorrectionProposalCore.FIELD, simpleName, senderDeclBinding, fieldRelevance, false); - T prop= newVariableCorrectionProposalToT(core, uid); - if (prop != null) - proposals.add(prop); + } else if (simpleName.getParent() instanceof InfixExpression infix) { + createEnumProposal= false; + if (infix.getOperator() == Operator.EQUALS || infix.getOperator() == Operator.NOT_EQUALS) { + Expression otherExp= null; + if (infix.getLeftOperand() == simpleName) { + otherExp= infix.getRightOperand(); + } else { + otherExp= infix.getLeftOperand(); + } + if (otherExp instanceof SimpleName otherIdent) { + IBinding otherBinding= otherIdent.resolveBinding(); + if (otherBinding instanceof IVariableBinding varBinding) { + if (!varBinding.isEnumConstant() && senderDeclBinding.isEqualTo(varBinding.getType())) { + createEnumProposal= true; + } + } + } + } + } else if (simpleName.getLocationInParent() == VariableDeclarationFragment.INITIALIZER_PROPERTY) { + createEnumProposal= false; + VariableDeclarationFragment fragment= (VariableDeclarationFragment)simpleName.getParent(); + IVariableBinding fragBinding= fragment.resolveBinding(); + if (fragBinding != null && senderDeclBinding.isEqualTo(fragBinding.getType())) { + createEnumProposal= true; + } + } else if (simpleName.getLocationInParent() == Assignment.RIGHT_HAND_SIDE_PROPERTY) { + createEnumProposal= false; + Assignment assignment= (Assignment) simpleName.getParent(); + Expression leftOperand= assignment.getLeftHandSide(); + if (senderDeclBinding.isEqualTo(leftOperand.resolveTypeBinding())) { + createEnumProposal= true; + } + } + if (createEnumProposal) { + label= Messages.format(CorrectionMessages.UnresolvedElementsSubProcessor_createenum_description, new Object[] { nameLabel, ASTResolving.getTypeSignature(senderDeclBinding) }); + NewVariableCorrectionProposalCore core= new NewVariableCorrectionProposalCore(label, targetCU, NewVariableCorrectionProposalCore.ENUM_CONST, simpleName, senderDeclBinding, 10, false); + T p1= newVariableCorrectionProposalToT(core, NewFieldForTypeProposal1); + if (p1 != null) + proposals.add(p1); } + } + if (!createEnumProposal && !mustBeConst) { + int fieldRelevance= StubUtility.hasFieldName(targetCU.getJavaProject(), name) ? IProposalRelevance.CREATE_FIELD_PREFIX_OR_SUFFIX_MATCH : IProposalRelevance.CREATE_FIELD; + int uid; + if (binding == null) { + label= Messages.format(CorrectionMessages.UnresolvedElementsSubProcessor_createfield_description, nameLabel); + uid= NewFieldForTypeProposal2; + } else { + label= Messages.format(CorrectionMessages.UnresolvedElementsSubProcessor_createfield_other_description, new Object[] { nameLabel, ASTResolving.getTypeSignature(senderDeclBinding) } ); + uid= NewFieldForTypeProposal3; + } + NewVariableCorrectionProposalCore core= new NewVariableCorrectionProposalCore(label, targetCU, NewVariableCorrectionProposalCore.FIELD, simpleName, senderDeclBinding, fieldRelevance, false); + T prop= newVariableCorrectionProposalToT(core, uid); + if (prop != null) + proposals.add(prop); + } - if (!isWriteAccess && !senderDeclBinding.isAnonymous()) { - int uid; - if (binding == null) { - label= Messages.format(CorrectionMessages.UnresolvedElementsSubProcessor_createconst_description, nameLabel); - uid= NewFieldForTypeProposal4; - } else { - label= Messages.format(CorrectionMessages.UnresolvedElementsSubProcessor_createconst_other_description, new Object[] { nameLabel, ASTResolving.getTypeSignature(senderDeclBinding) } ); - uid= NewFieldForTypeProposal5; - } - int constRelevance= StubUtility.hasConstantName(targetCU.getJavaProject(), name) ? IProposalRelevance.CREATE_CONSTANT_PREFIX_OR_SUFFIX_MATCH : IProposalRelevance.CREATE_CONSTANT; - NewVariableCorrectionProposalCore core= new NewVariableCorrectionProposalCore(label, targetCU, NewVariableCorrectionProposalCore.CONST_FIELD, simpleName, senderDeclBinding, constRelevance, false); - T prop= newVariableCorrectionProposalToT(core, uid); - if (prop != null) - proposals.add(prop); + if (!createEnumProposal && !isWriteAccess && !senderDeclBinding.isAnonymous()) { + int uid; + if (binding == null) { + label= Messages.format(CorrectionMessages.UnresolvedElementsSubProcessor_createconst_description, nameLabel); + uid= NewFieldForTypeProposal4; + } else { + label= Messages.format(CorrectionMessages.UnresolvedElementsSubProcessor_createconst_other_description, new Object[] { nameLabel, ASTResolving.getTypeSignature(senderDeclBinding) } ); + uid= NewFieldForTypeProposal5; } + int constRelevance= StubUtility.hasConstantName(targetCU.getJavaProject(), name) ? IProposalRelevance.CREATE_CONSTANT_PREFIX_OR_SUFFIX_MATCH : IProposalRelevance.CREATE_CONSTANT; + NewVariableCorrectionProposalCore core= new NewVariableCorrectionProposalCore(label, targetCU, NewVariableCorrectionProposalCore.CONST_FIELD, simpleName, senderDeclBinding, constRelevance, false); + T prop= newVariableCorrectionProposalToT(core, uid); + if (prop != null) + proposals.add(prop); } } diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/UnresolvedVariablesQuickFixTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/UnresolvedVariablesQuickFixTest.java index 106f5d004f5..ff9a698cb3b 100644 --- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/UnresolvedVariablesQuickFixTest.java +++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/UnresolvedVariablesQuickFixTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2020 IBM Corporation and others. + * Copyright (c) 2000, 2024 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -2265,6 +2265,324 @@ void foo(Colors c) { assertEqualStringsIgnoreOrder(new String[] { preview1, preview2 }, new String[] { expected1, expected2 }); } + @Test + public void testVarInEnumMethodReturn1() throws Exception { + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + String str= """ + package test1; + public enum Colors { + RED; + public Colors foo() { + return BLUE; + } + } + """; + ICompilationUnit cu= pack1.createCompilationUnit("Colors.java", str, false, null); + + CompilationUnit astRoot= getASTRoot(cu); + ArrayList proposals= collectCorrections(cu, astRoot); + assertNumberOfProposals(proposals, 4); + assertCorrectLabels(proposals); + + CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0); + String preview1= getPreviewContent(proposal); + + String expected1= """ + package test1; + public enum Colors { + RED, BLUE; + public Colors foo() { + return BLUE; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(1); + String preview2= getPreviewContent(proposal); + + String expected2= """ + package test1; + public enum Colors { + RED; + public Colors foo() { + return RED; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(2); + String preview3= getPreviewContent(proposal); + + String expected3= """ + package test1; + public enum Colors { + RED; + public Colors foo() { + Colors BLUE; + return BLUE; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(3); + String preview4= getPreviewContent(proposal); + + String expected4= """ + package test1; + public enum Colors { + RED; + public Colors foo(Colors BLUE) { + return BLUE; + } + } + """; + + assertEqualStringsIgnoreOrder(new String[] { preview1, preview2, preview3, preview4 }, new String[] { expected1, expected2, expected3, expected4 }); + } + + @Test + public void testVarInEnumMethodReturn2() throws Exception { + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + String str= """ + package test1; + public enum Colors { + RED; + public int foo() { + return BLUE; + } + } + """; + ICompilationUnit cu= pack1.createCompilationUnit("Colors.java", str, false, null); + + CompilationUnit astRoot= getASTRoot(cu); + ArrayList proposals= collectCorrections(cu, astRoot); + assertNumberOfProposals(proposals, 4); + assertCorrectLabels(proposals); + + CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0); + String preview1= getPreviewContent(proposal); + + String expected1= """ + package test1; + public enum Colors { + RED; + private int BLUE; + + public int foo() { + return BLUE; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(1); + String preview2= getPreviewContent(proposal); + + String expected2= """ + package test1; + public enum Colors { + RED; + private static final int BLUE = 0; + + public int foo() { + return BLUE; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(2); + String preview3= getPreviewContent(proposal); + + String expected3= """ + package test1; + public enum Colors { + RED; + public int foo() { + int BLUE; + return BLUE; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(3); + String preview4= getPreviewContent(proposal); + + String expected4= """ + package test1; + public enum Colors { + RED; + public int foo(int BLUE) { + return BLUE; + } + } + """; + + assertEqualStringsIgnoreOrder(new String[] { preview1, preview2, preview3, preview4 }, new String[] { expected1, expected2, expected3, expected4 }); + } + + @Test + public void testVarInEnumMethod1() throws Exception { + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + String str= """ + package test1; + public enum Colors { + RED; + public Colors foo() { + Colors k = BLUE; + return k; + } + } + """; + ICompilationUnit cu= pack1.createCompilationUnit("Colors.java", str, false, null); + + CompilationUnit astRoot= getASTRoot(cu); + ArrayList proposals= collectCorrections(cu, astRoot); + assertNumberOfProposals(proposals, 4); + assertCorrectLabels(proposals); + + CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0); + String preview1= getPreviewContent(proposal); + + String expected1= """ + package test1; + public enum Colors { + RED, BLUE; + public Colors foo() { + Colors k = BLUE; + return k; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(1); + String preview2= getPreviewContent(proposal); + + String expected2= """ + package test1; + public enum Colors { + RED; + public Colors foo() { + Colors BLUE; + Colors k = BLUE; + return k; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(2); + String preview3= getPreviewContent(proposal); + + String expected3= """ + package test1; + public enum Colors { + RED; + public Colors foo() { + Colors k = RED; + return k; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(3); + String preview4= getPreviewContent(proposal); + + String expected4= """ + package test1; + public enum Colors { + RED; + public Colors foo(Colors BLUE) { + Colors k = BLUE; + return k; + } + } + """; + + assertEqualStringsIgnoreOrder(new String[] { preview1, preview2, preview3, preview4 }, new String[] { expected1, expected2, expected3, expected4 }); + } + + @Test + public void testVarInEnumMethod2() throws Exception { + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + String str= """ + package test1; + public enum Colors { + RED; + public int foo() { + int k = BLUE; + return k; + } + } + """; + ICompilationUnit cu= pack1.createCompilationUnit("Colors.java", str, false, null); + + CompilationUnit astRoot= getASTRoot(cu); + ArrayList proposals= collectCorrections(cu, astRoot); + assertNumberOfProposals(proposals, 4); + assertCorrectLabels(proposals); + + CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0); + String preview1= getPreviewContent(proposal); + + String expected1= """ + package test1; + public enum Colors { + RED; + private int BLUE; + + public int foo() { + int k = BLUE; + return k; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(1); + String preview2= getPreviewContent(proposal); + + String expected2= """ + package test1; + public enum Colors { + RED; + private static final int BLUE = 0; + + public int foo() { + int k = BLUE; + return k; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(2); + String preview3= getPreviewContent(proposal); + + String expected3= """ + package test1; + public enum Colors { + RED; + public int foo() { + int BLUE; + int k = BLUE; + return k; + } + } + """; + + proposal= (CUCorrectionProposal) proposals.get(3); + String preview4= getPreviewContent(proposal); + + String expected4= """ + package test1; + public enum Colors { + RED; + public int foo(int BLUE) { + int k = BLUE; + return k; + } + } + """; + + assertEqualStringsIgnoreOrder(new String[] { preview1, preview2, preview3, preview4 }, new String[] { expected1, expected2, expected3, expected4 }); + } + @Test public void testVarInMethodInvocation() throws Exception { IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);