diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/OperatorOverloadingInferencingTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/OperatorOverloadingInferencingTests.java index e1f76eca4f..5146aba513 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/OperatorOverloadingInferencingTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/OperatorOverloadingInferencingTests.java @@ -336,7 +336,7 @@ public void testLongExpr3() throws Exception { } @Test - public void testNumberPlusString1() throws Exception { + public void testNumberPlusString() throws Exception { String contents = "def xxx = 1 + ''\n" + "xxx"; @@ -345,7 +345,7 @@ public void testNumberPlusString1() throws Exception { } @Test - public void testNumberPlusString2() throws Exception { + public void testNumberPlusGString() throws Exception { String contents = "def xxx = 1 + \"${this}\"\n" + "xxx"; diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java index 8bd82cb98e..0cfdadcfa5 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java @@ -243,8 +243,7 @@ protected TypeLookupResult findType(final Expression node, final ClassNode decla return new TypeLookupResult(VariableScope.BOOLEAN_CLASS_NODE, null, null, confidence, scope); } else if (node instanceof GStringExpression) { - // return String not GString so that DGMs will apply - return new TypeLookupResult(VariableScope.STRING_CLASS_NODE, null, null, confidence, scope); + return new TypeLookupResult(VariableScope.GSTRING_CLASS_NODE, null, null, confidence, scope); } else if (node instanceof CastExpression) { return new TypeLookupResult(node.getType(), null, null, confidence, scope); diff --git a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ExtractMethodTests.groovy b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ExtractMethodTests.groovy index 1f43c02e59..52e61998ef 100644 --- a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ExtractMethodTests.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ExtractMethodTests.groovy @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * https://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -27,6 +27,7 @@ import org.eclipse.jdt.core.Flags import org.eclipse.ltk.core.refactoring.Change import org.eclipse.ltk.core.refactoring.RefactoringStatus import org.eclipse.ltk.core.refactoring.RefactoringStatusEntry +import org.junit.Assert import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Parameterized @@ -36,13 +37,11 @@ import org.junit.runners.Parameterized.Parameters final class ExtractMethodTests extends GroovyEclipseTestSuite { @Parameters - static Iterable params() { + static params() { URL url = Platform.getBundle('org.codehaus.groovy.eclipse.refactoring.test').getEntry('/resources/ExtractMethod') - new File(FileLocator.toFileURL(url).getFile()).listFiles({ File dir, String item -> + new File(FileLocator.toFileURL(url).file).listFiles({ File dir, String item -> item ==~ /ExtractMethod_Test_.*/ - } as FilenameFilter).collect { - [it] as Object[] - } + } as FilenameFilter) } ExtractMethodTests(File file) { @@ -66,7 +65,7 @@ final class ExtractMethodTests extends GroovyEclipseTestSuite { String actual = String.valueOf(unit.contents) String expect = spec.expected.get() - assert actual == expect + Assert.assertEquals(expect, actual) } private void preAction() { @@ -75,12 +74,13 @@ final class ExtractMethodTests extends GroovyEclipseTestSuite { int offset = spec.userSelection.offset int length = spec.userSelection.length RefactoringStatus status = new RefactoringStatus() - printf 'Attempting to extract new method from [%d,%d):%n %s%n', offset, offset + length, String.valueOf(unit.contents).substring(offset, offset + length) + printf 'Attempting to extract new method from [%d,%d):%n %s%n', + offset, offset + length, String.valueOf(unit.contents).substring(offset, offset + length) refactoring = new ExtractGroovyMethodRefactoring(unit, offset, length, status) refactoring.setPreferences(TestPrefInitializer.initializePreferences(spec.properties as HashMap, unit.javaProject)) - assert status.getSeverity() == RefactoringStatus.OK : "Bad refactoring status on init: $status" + assert status.severity == RefactoringStatus.OK : "Bad refactoring status on init: $status" } private RefactoringStatus checkInitialCondition() { @@ -121,13 +121,13 @@ final class ExtractMethodTests extends GroovyEclipseTestSuite { String variableToRename = spec.properties['variableToRename'] if (variableToRename != null && variableToRename.trim().length() > 0) { Map variablesToRename = [:] - for (String renameMapping : variableToRename.split(';')) { + for (renameMapping in variableToRename.split(';')) { String[] singleRenames = renameMapping.split(':') if (singleRenames.length == 2) { variablesToRename.put(singleRenames[0], singleRenames[1]) } } - refactoring.setParameterRename(variablesToRename) + refactoring.parameterRename = variablesToRename } } @@ -136,11 +136,11 @@ final class ExtractMethodTests extends GroovyEclipseTestSuite { } private boolean analyseRefactoringStatus(RefactoringStatus state) { - RefactoringStatusEntry[] entries = state.getEntries() + RefactoringStatusEntry[] entries = state.entries if (spec.shouldFail) { assert entries.length > 0 : "Should fail: ${spec.properties['failMessage']}" } - for (int i = 0; i < entries.length; i++) { + for (int i = 0; i < entries.length; i += 1) { RefactoringStatusEntry entry = entries[i] if ((entry.isError() || entry.isFatalError()) && !spec.shouldFail) { // error was not expected diff --git a/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ExtractGroovyMethodRefactoring.java b/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ExtractGroovyMethodRefactoring.java index 3a4a573cce..3a2933800b 100644 --- a/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ExtractGroovyMethodRefactoring.java +++ b/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ExtractGroovyMethodRefactoring.java @@ -22,6 +22,7 @@ import java.util.Map; import java.util.Observable; import java.util.Observer; +import java.util.Optional; import java.util.Set; import java.util.StringTokenizer; @@ -96,12 +97,12 @@ public class ExtractGroovyMethodRefactoring extends Refactoring { private int newMethodModifier = Flags.AccDefault; /** - * Text that will be replaced by the refactoring + * Text that will be replaced by the refactoring. */ private Region replaceScope; /** - * Text that is currently selected + * Text that is currently selected. */ private Region selectedText; @@ -109,20 +110,13 @@ public class ExtractGroovyMethodRefactoring extends Refactoring { private boolean returnMustBeDeclared; - /** - * Two collections since the variables in the methodCall - * and in the signature of the method can be different - */ private List actualParameters; private List inferredTypeOfActualParameters; private List originalParametersBeforeRename; - /** - * Although we can determine if there are multiple return parameters - * we only support on return parameter - */ + // Although we can determine if there are multiple return parameters we only support on return parameter. private Set returnParameters; private List inferredReturnTypes; @@ -135,21 +129,21 @@ public class ExtractGroovyMethodRefactoring extends Refactoring { private CompilationUnitChange change; - private GroovyRefactoringObservable observable = new GroovyRefactoringObservable(); + private final GroovyRefactoringObservable observable = new GroovyRefactoringObservable(); - public ExtractGroovyMethodRefactoring(GroovyCompilationUnit unit, int offset, int length, RefactoringStatus status) { + public ExtractGroovyMethodRefactoring(final GroovyCompilationUnit unit, final int offset, final int length, final RefactoringStatus status) { this.unit = unit; this.selectedText = new Region(offset, length); this.refactoringPreferences = Activator.getDefault().getPreferenceStore(); initializeExtractedStatements(status); } - public ExtractGroovyMethodRefactoring(JavaRefactoringArguments arguments, RefactoringStatus status) { + public ExtractGroovyMethodRefactoring(final JavaRefactoringArguments arguments, final RefactoringStatus status) { status.merge(initialize(arguments)); initializeExtractedStatements(status); } - private void initializeExtractedStatements(RefactoringStatus status) { + private void initializeExtractedStatements(final RefactoringStatus status) { try { methodCodeFinder = new StatementFinder(selectedText, unit.getModuleNode()); createBlockStatement(); @@ -162,7 +156,7 @@ private void initializeExtractedStatements(RefactoringStatus status) { } @Override - public RefactoringStatus checkInitialConditions(IProgressMonitor monitor) throws CoreException, OperationCanceledException { + public RefactoringStatus checkInitialConditions(final IProgressMonitor monitor) throws CoreException, OperationCanceledException { RefactoringStatus status = new RefactoringStatus(); monitor.beginTask("Checking initial conditions for extract method", 100); @@ -192,9 +186,9 @@ public RefactoringStatus checkInitialConditions(IProgressMonitor monitor) throws } @Override - public RefactoringStatus checkFinalConditions(IProgressMonitor pm) throws CoreException, OperationCanceledException { + public RefactoringStatus checkFinalConditions(final IProgressMonitor monitor) throws CoreException, OperationCanceledException { RefactoringStatus stat = new RefactoringStatus(); - stat.merge(checkDuplicateMethod(pm)); + stat.merge(checkDuplicateMethod(monitor)); change = new CompilationUnitChange(GroovyRefactoringMessages.ExtractMethodRefactoring, unit); change.setEdit(new MultiTextEdit()); @@ -212,7 +206,7 @@ public RefactoringStatus checkFinalConditions(IProgressMonitor pm) throws CoreEx } @Override - public Change createChange(IProgressMonitor pm) throws CoreException, OperationCanceledException { + public Change createChange(final IProgressMonitor monitor) throws CoreException, OperationCanceledException { return change; } @@ -224,11 +218,11 @@ public String getName() { /** * For testing, override actual preferences with test-specific ones */ - public void setPreferences(IPreferenceStore preferences) { + public void setPreferences(final IPreferenceStore preferences) { this.refactoringPreferences = preferences; } - private RefactoringStatus initialize(JavaRefactoringArguments arguments) { + private RefactoringStatus initialize(final JavaRefactoringArguments arguments) { final String selection = arguments.getAttribute(JavaRefactoringDescriptorUtil.ATTRIBUTE_SELECTION); if (selection == null) { return RefactoringStatus.createFatalErrorStatus(Messages.format( @@ -280,11 +274,11 @@ private void saveOriginalParameters() { } } - public void addObserver(Observer observer) { + public void addObserver(final Observer observer) { observable.addObserver(observer); } - public void setNewMethodname(String newMethodname) { + public void setNewMethodname(final String newMethodname) { this.newMethodName = newMethodname; updateMethod(); observable.setChanged(); @@ -295,7 +289,7 @@ public String getNewMethodName() { return newMethodName; } - public void setModifier(int modifier) { + public void setModifier(final int modifier) { this.newMethodModifier = modifier; updateMethod(); observable.setChanged(); @@ -306,7 +300,7 @@ public int getModifier() { return newMethodModifier; } - private void setCallAndMethHeadParameters(List params) { + private void setCallAndMethHeadParameters(final List params) { actualParameters = params; inferredTypeOfActualParameters.clear(); for (Variable variable : params) { @@ -332,13 +326,12 @@ public Parameter[] getCallAndMethHeadParameters() { * @return String containing the call */ public String getMethodCall() { - Expression objExp = new VariableExpression("this"); ArgumentListExpression arguments = new ArgumentListExpression(); for (Variable param : originalParametersBeforeRename) { arguments.addExpression(new VariableExpression(param.getName(), - param.getOriginType() == null ? ClassHelper.DYNAMIC_TYPE : param.getOriginType())); + Optional.ofNullable(param.getOriginType()).orElse(VariableScope.OBJECT_CLASS_NODE))); } MethodCallExpression newMethodCall = new MethodCallExpression(objExp, newMethodName, arguments); @@ -354,7 +347,7 @@ public String getMethodCall() { return writer.getGroovyCode(); } - private void visitExpressionsForReturnStmt(MethodCallExpression newMethodCall, ASTWriter astw) { + private void visitExpressionsForReturnStmt(final MethodCallExpression newMethodCall, final ASTWriter astw) { Assert.isTrue(!returnParameters.isEmpty()); Variable retVar = returnParameters.iterator().next(); if (returnMustBeDeclared) { @@ -367,11 +360,6 @@ private void visitExpressionsForReturnStmt(MethodCallExpression newMethodCall, A } } - /** - * Return a method head for preview - * - * @return - */ public String getMethodHead() { updateMethod(); @@ -383,19 +371,15 @@ public String getMethodHead() { return head.substring(0, headEndPos + 1).trim(); } - /** - * create the method node with all given parameters - */ private void updateMethod() { // rearrange parameters if necessary if (!block.getStatements().isEmpty()) { Parameter[] params = getCallAndMethHeadParameters(); - ClassNode returnType = ClassHelper.DYNAMIC_TYPE; + ClassNode returnType; if (!returnParameters.isEmpty()) { returnType = inferredReturnTypes.get(0); - if (returnType.equals(VariableScope.OBJECT_CLASS_NODE)) { - returnType = ClassHelper.DYNAMIC_TYPE; - } + } else { + returnType = VariableScope.OBJECT_CLASS_NODE; } newMethod = new MethodNode(newMethodName, 0, returnType, params, null, block); @@ -467,42 +451,41 @@ private void defineActualAndReturnParameters() { } // now try to infer the variable types - InferParameterAndReturnTypesRequestor inferRequestor = new InferParameterAndReturnTypesRequestor(actualParameters, - returnParameters, selectedText); + InferParameterAndReturnTypesRequestor requestor = new InferParameterAndReturnTypesRequestor(actualParameters, returnParameters, selectedText); TypeInferencingVisitorWithRequestor visitor = new TypeInferencingVisitorFactory().createVisitor(unit); - visitor.visitCompilationUnit(inferRequestor); + visitor.visitCompilationUnit(requestor); - Map inferredTypes = inferRequestor.getInferredTypes(); + Map inferredTypes = requestor.getInferredTypes(); for (Variable variable : actualParameters) { - if (inferredTypes.containsKey(variable)) { - ClassNode type = inferredTypes.get(variable); - if (type == null || VariableScope.isVoidOrObject(type)) { - inferredTypeOfActualParameters.add(ClassHelper.DYNAMIC_TYPE); - } else { - // force using a cached type so that getUnwrapper will work - inferredTypeOfActualParameters.add(maybeConvertToPrimitiveType(type)); - } - } else { - inferredTypeOfActualParameters.add(ClassHelper.DYNAMIC_TYPE); - } + ClassNode type = Optional.ofNullable(inferredTypes.get(variable)).filter(t -> !VariableScope.isVoidOrObject(t)) + .map(ExtractGroovyMethodRefactoring::normalizeInferredType) + .orElse(VariableScope.OBJECT_CLASS_NODE); + inferredTypeOfActualParameters.add(type); } for (Variable variable : returnParameters) { - if (inferredTypes.containsKey(variable)) { - // force using a cached type so that getUnwrapper will work - inferredReturnTypes.add(maybeConvertToPrimitiveType(inferredTypes.get(variable))); - } else { - inferredReturnTypes.add(variable.getOriginType()); - } + ClassNode type = Optional.ofNullable(inferredTypes.get(variable)) + .map(ExtractGroovyMethodRefactoring::normalizeInferredType) + .orElse(VariableScope.OBJECT_CLASS_NODE); + inferredReturnTypes.add(type); } } - private ClassNode maybeConvertToPrimitiveType(ClassNode type) { - return ClassHelper.getUnwrapper(type).getPlainNodeReference(); + private static ClassNode normalizeInferredType(final ClassNode t) { + if (t.equals(VariableScope.GSTRING_CLASS_NODE)) { + return VariableScope.STRING_CLASS_NODE; + } + if (VariableScope.isVoidOrObject(t)) { + return t.redirect(); + } + if (ClassHelper.isPrimitiveType(t)) { + return t; + } + return ClassHelper.getUnwrapper(t).getPlainNodeReference(); } - private RefactoringStatus checkDuplicateMethod(IProgressMonitor monitor) { - monitor = SubMonitor.convert(monitor, "Checking for duplicate methods", 25); + private RefactoringStatus checkDuplicateMethod(final IProgressMonitor monitor) { + SubMonitor.convert(monitor, "Checking for duplicate methods", 25); RefactoringStatus stat = new RefactoringStatus(); if (getMethodNames().contains(newMethodName)) { Object[] message = {newMethodName, getClassName()}; @@ -512,8 +495,8 @@ private RefactoringStatus checkDuplicateMethod(IProgressMonitor monitor) { return stat; } - private RefactoringStatus checkExtractFromConstructor(IProgressMonitor monitor) { - monitor = SubMonitor.convert(monitor, "Checking for constructor calls", 25); + private RefactoringStatus checkExtractFromConstructor(final IProgressMonitor monitor) { + SubMonitor.convert(monitor, "Checking for constructor calls", 25); RefactoringStatus stat = new RefactoringStatus(); if (methodCodeFinder.isInConstructor()) { if (new ExtractConstructorTest().containsConstructorCall(newMethod)) { @@ -524,8 +507,8 @@ private RefactoringStatus checkExtractFromConstructor(IProgressMonitor monitor) return stat; } - private RefactoringStatus checkStatementSelection(IProgressMonitor monitor) { - monitor = SubMonitor.convert(monitor, "Checking statement selection", 25); + private RefactoringStatus checkStatementSelection(final IProgressMonitor monitor) { + SubMonitor.convert(monitor, "Checking statement selection", 25); RefactoringStatus stat = new RefactoringStatus(); int selectionLength = selectedText.getLength(); if (block.isEmpty() && selectionLength >= 0) { @@ -534,8 +517,8 @@ private RefactoringStatus checkStatementSelection(IProgressMonitor monitor) { return stat; } - private RefactoringStatus checkNrOfReturnValues(IProgressMonitor monitor) { - monitor = SubMonitor.convert(monitor, "Checking number of return values", 25); + private RefactoringStatus checkNrOfReturnValues(final IProgressMonitor monitor) { + SubMonitor.convert(monitor, "Checking number of return values", 25); RefactoringStatus stat = new RefactoringStatus(); if (returnParameters != null && returnParameters.size() > 1) { StringBuilder retValues = new StringBuilder(); @@ -551,7 +534,7 @@ private RefactoringStatus checkNrOfReturnValues(IProgressMonitor monitor) { /** * Returns the Code of the new Method as a formated IDocument. */ - private String createCopiedMethodCode(RefactoringStatus status) { + private String createCopiedMethodCode(final RefactoringStatus status) { IDocument unitDocument = new Document(String.valueOf(unit.getContents())); String lineDelimiter = TextUtilities.getDefaultLineDelimiter(unitDocument); @@ -593,7 +576,7 @@ private String createCopiedMethodCode(RefactoringStatus status) { /** * @return may return null if there is a parse problem */ - private MethodNode createNewMethodForValidation(String methodText, RefactoringStatus status) { + private MethodNode createNewMethodForValidation(final String methodText, final RefactoringStatus status) { try { GroovySnippetParser parser = new GroovySnippetParser(); ModuleNode module = parser.parse(methodText); @@ -642,12 +625,12 @@ private int calculateIndentation() { return defaultIndentation; } - private MultiTextEdit renameVariableInExtractedMethod(MethodNode method) { + private MultiTextEdit renameVariableInExtractedMethod(final MethodNode method) { VariableRenamer renamer = new VariableRenamer(); return renamer.rename(method, variablesToRename); } - private ASTWriter writeReturnStatements(IDocument document) { + private ASTWriter writeReturnStatements(final IDocument document) { ASTWriter astw = new ASTWriter(unit.getModuleNode(), document); for (Variable var : returnParameters) { ReturnStatement ret = new ReturnStatement(new VariableExpression(var)); @@ -657,7 +640,7 @@ private ASTWriter writeReturnStatements(IDocument document) { return astw; } - private InsertEdit createMethodDeclarationEdit(RefactoringStatus status) { + private InsertEdit createMethodDeclarationEdit(final RefactoringStatus status) { return new InsertEdit(methodCodeFinder.getSelectedDeclaration().getEnd(), createCopiedMethodCode(status)); } @@ -674,12 +657,12 @@ public String getClassName() { } /** - * @param variName + * @param variName TODO * @param upEvent true if the move is upwards * @param numberOfMoves mostly 1, can be more for tests * @return the index of the selected variable in the collection */ - public int setMoveParameter(String variName, boolean upEvent, int numberOfMoves) { + public int setMoveParameter(final String variName, final boolean upEvent, final int numberOfMoves) { Parameter[] originalParams = getCallAndMethHeadParameters(); List newParamList = new ArrayList<>(); @@ -698,7 +681,7 @@ public int setMoveParameter(String variName, boolean upEvent, int numberOfMoves) return indexOfSelectedParam; } - private int reorderParameters(boolean upEvent, int numberOfMoves, List newParamList, int index) { + private int reorderParameters(final boolean upEvent, final int numberOfMoves, final List newParamList, final int index) { int indexOfSelectedParam = index; // also reorder in originals! Variable variToMove = newParamList.remove(indexOfSelectedParam); @@ -711,7 +694,7 @@ private int reorderParameters(boolean upEvent, int numberOfMoves, List return indexOfSelectedParam; } - private int calculateNewIndexAfterMove(boolean upEvent, int numberOfMoves, List newParamList, int index) { + private int calculateNewIndexAfterMove(final boolean upEvent, final int numberOfMoves, final List newParamList, final int index) { int indexOfSelectedParam = index; if (upEvent) { if (indexOfSelectedParam < 1) { @@ -729,7 +712,7 @@ private int calculateNewIndexAfterMove(boolean upEvent, int numberOfMoves, List< return indexOfSelectedParam; } - public void setParameterRename(Map variablesToRename) { + public void setParameterRename(final Map variablesToRename) { this.variablesToRename = variablesToRename; List newParamList = new ArrayList<>(); @@ -746,7 +729,7 @@ public void setParameterRename(Map variablesToRename) { observable.notifyObservers(); } - public String getOriginalParameterName(int selectionIndex) { + public String getOriginalParameterName(final int selectionIndex) { return originalParametersBeforeRename.get(selectionIndex).getName(); }