diff --git a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ConvertLocalToFieldTests.groovy b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ConvertLocalToFieldTests.groovy index 71f30baba9..8f55d38769 100644 --- a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ConvertLocalToFieldTests.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ConvertLocalToFieldTests.groovy @@ -1,5 +1,5 @@ /* - * Copyright 2009-2017 the original author or authors. + * Copyright 2009-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ */ package org.codehaus.groovy.eclipse.refactoring.test.extract +import groovy.transform.NotYetImplemented + import org.codehaus.groovy.eclipse.refactoring.core.extract.ConvertGroovyLocalToFieldRefactoring import org.codehaus.groovy.eclipse.refactoring.test.RefactoringTestSuite import org.codehaus.jdt.groovy.model.GroovyCompilationUnit @@ -30,8 +32,11 @@ final class ConvertLocalToFieldTests extends RefactoringTestSuite { null } - private void runTest(String testName) { + private void runTest(String testName = test.methodName) { ConvertLocalToFieldTestsData.TestCase testCase = ConvertLocalToFieldTestsData.getTestCases().get(testName) + } + + private void runTest(ConvertLocalToFieldTestsData.TestCase testCase) { GroovyCompilationUnit cu = (GroovyCompilationUnit) createCU(packageP, 'Test.groovy', testCase.getInput()) ConvertGroovyLocalToFieldRefactoring refactoring = @@ -70,6 +75,8 @@ final class ConvertLocalToFieldTests extends RefactoringTestSuite { assertEqualLines('invalid redo', testCase.getExpected(), cu.getSource()) } + //-------------------------------------------------------------------------- + @Test void testMethodToModule() { runTest('testMethodToModule') @@ -177,21 +184,115 @@ final class ConvertLocalToFieldTests extends RefactoringTestSuite { @Test void testClosure() { - runTest('testClosure') + runTest() } @Test void testClosureVariableConflict() { - runTest('testClosureVariableConflict') + runTest() } @Test void testClosureParameterList() { - runTest('testClosureParameterList') + runTest() } @Test void testClosureImplicitIt() { - runTest('testClosureImplicitIt') + runTest() + } + + @Test @NotYetImplemented + void testWithinFieldInitializer1() { + def testCase = new ConvertLocalToFieldTestsData.TestCase('''\ + class Pogo { + private Object field = { -> + def target/**/ = null + } + } + '''.stripIndent(), '''\ + class Pogo { + \tprivate def target + private Object field = { -> + target/**/ = null + } + } + '''.stripIndent()) + runTest(testCase) + } + + @Test + void testWithinFieldInitializer2() { + def testCase = new ConvertLocalToFieldTestsData.TestCase('''\ + @groovy.transform.Field + Object field = { -> + def target/**/ = null + } + '''.stripIndent(), '''\ + @groovy.transform.Field def target + @groovy.transform.Field + Object field = { -> + target/**/ = null + } + '''.stripIndent()) + runTest(testCase) + } + + @Test + void testWithinObjectInitializer() { + def testCase = new ConvertLocalToFieldTestsData.TestCase('''\ + class Pogo { + { + def target/**/ = null + } + } + '''.stripIndent(), '''\ + class Pogo { + \tprivate def target + { + target/**/ = null + } + } + '''.stripIndent()) + runTest(testCase) + } + + @Test @NotYetImplemented + void testWithinStaticInitializer() { + def testCase = new ConvertLocalToFieldTestsData.TestCase('''\ + class Pogo { + static { + def target/**/ = null + } + } + '''.stripIndent(), '''\ + class Pogo { + \tprivate static def target + static { + target/**/ = null + } + } + '''.stripIndent()) + runTest(testCase) + } + + @Test @NotYetImplemented // not sure if this is legal, but wanted case to test enclosing checks + void testWithinAnnotationClosure() { + createCU(packageP, 'Tag.groovy', '@interface Tag { Class value() }') + def testCase = new ConvertLocalToFieldTestsData.TestCase('''\ + @Tag(value = { -> + def target/**/ = null + }) + class Pogo { + } + '''.stripIndent(), '''\ + @Tag(value = { -> + Pogo.target/**/ = null + }) + class Pogo { + \tprivate static def target + } + '''.stripIndent()) + runTest(testCase) } } diff --git a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ConvertLocalToFieldTestsData.groovy b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ConvertLocalToFieldTestsData.groovy index d775bc68da..de52794cf5 100644 --- a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ConvertLocalToFieldTestsData.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/extract/ConvertLocalToFieldTestsData.groovy @@ -1,5 +1,5 @@ /* - * Copyright 2009-2016 the original author or authors. + * Copyright 2009-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,15 +23,15 @@ class ConvertLocalToFieldTestsData { String fieldName boolean expectWarning - public TestCase(input, expected = null, fieldName = "target", expectWarning = false) { + public TestCase(String input, String expected = null, String fieldName = "target", boolean expectWarning = false) { this.input = input this.expected = expected this.fieldName = fieldName this.expectWarning = expectWarning } - int getSelectionOffset() { return input.indexOf("target/**/") } - int getSelectionLength() { return "target".length() } + int getSelectionOffset() { input.indexOf('target/**/') } + int getSelectionLength() { 'target'.length() } } static Map testCases = [ diff --git a/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ConvertGroovyLocalToFieldRefactoring.java b/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ConvertGroovyLocalToFieldRefactoring.java index 1e5433e7bb..5c209b9b67 100644 --- a/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ConvertGroovyLocalToFieldRefactoring.java +++ b/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ConvertGroovyLocalToFieldRefactoring.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2017 the original author or authors. + * Copyright 2009-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -50,6 +50,7 @@ import org.codehaus.jdt.groovy.model.GroovyCompilationUnit; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.OperationCanceledException; import org.eclipse.jdt.core.compiler.CharOperation; import org.eclipse.jdt.core.refactoring.CompilationUnitChange; import org.eclipse.jdt.groovy.core.util.DepthFirstVisitor; @@ -90,7 +91,6 @@ public class ConvertGroovyLocalToFieldRefactoring extends PromoteTempToFieldRefa private DeclarationExpression declarationExpression; private ClassNode containingClassNode; private ModuleNode moduleNode; - private MethodNode methodNode; private CompilationUnitChange change; @@ -247,12 +247,12 @@ public RefactoringStatus checkInitialConditions(IProgressMonitor monitor) throws this.variableExpressionInDeclaration = variableExpressionInDeclaration; - ClassNode containingClassNode = getContainingClassNode(); - if (containingClassNode == null) { + ClassNode containingClass = getContainingClassNode(); + if (containingClass == null) { result.merge(RefactoringStatus.createFatalErrorStatus("Cannot find enclosing class declaration.")); return result; } - if (containingClassNode.isInterface() || containingClassNode.isAnnotationDefinition()) { + if (containingClass.isInterface() || containingClass.isAnnotationDefinition()) { result.merge(RefactoringStatus.createFatalErrorStatus("Cannot add field to an interface or annotation definition.")); return result; } @@ -311,32 +311,36 @@ public RefactoringStatus checkFinalConditions(IProgressMonitor monitor) throws C this.change = change; return status; + } catch (BadLocationException e) { + throw new OperationCanceledException(e.getMessage()); } finally { monitor.done(); } } - private TextEditGroup createFieldTextEditGroup() { + private TextEditGroup createFieldTextEditGroup() throws BadLocationException { TextEdit textEdit = null; - ClassNode classNode = getContainingClassNode(); - MethodNode methodNode = getContainingMethodNode(); - if (methodNode.isScriptBody() && getContainingClosureExpression() == null) { + ClassNode containingClass = getContainingClassNode(); + MethodNode containingMethod = getContainingMethodNode(); + if (containingMethod != null && containingMethod.isScriptBody() && getContainingClosureExpression() == null) { textEdit = new InsertEdit(getDeclarationOffset(), "@groovy.transform.Field "); + } else if (containingClass != null && containingClass.isScript()) { + textEdit = new InsertEdit(containingClass.getStart(), createFieldText(0) + ASTTools.getLineDelimeter(unit)); } else { - try { - char[] contents = unit.getContents(); - int methodOffset = methodNode.getStart(); - int methodLineOffset = methodNode.getStart() - methodNode.getColumnNumber() + 1; - int insertOffset = classNode.isScript() ? classNode.getStart() : CharOperation.indexOf('{', contents, classNode.getStart()) + 1; + char[] contents = unit.getContents(); + int indentationLevel = 1; + if (containingMethod != null && containingMethod.getEnd() > 0) { + int methodLineOffset = containingMethod.getStart() - containingMethod.getColumnNumber() + 1; + String methodIndentation = String.valueOf(CharOperation.subarray(contents, methodLineOffset, containingMethod.getStart())); + indentationLevel = ASTTools.getCurrentIntentation(methodIndentation); + } - String methodIndentation = String.valueOf(CharOperation.subarray(contents, methodLineOffset, methodOffset)); - String fieldText = createFieldText(ASTTools.getCurrentIntentation(methodIndentation)); - String newline = ASTTools.getLineDelimeter(unit); + int insertOffset = CharOperation.indexOf('{', contents, containingClass.getStart()) + 1; + String fieldText = createFieldText(indentationLevel); + String newline = ASTTools.getLineDelimeter(unit); - textEdit = new InsertEdit(insertOffset, classNode.isScript() ? fieldText + newline : newline + fieldText); - } catch (Exception e) { - } + textEdit = new InsertEdit(insertOffset, newline + fieldText); } TextEditGroup group = new TextEditGroup("Create field."); @@ -356,7 +360,7 @@ private String createFieldText(int indentLevel) throws BadLocationException, Mal sb.append("@groovy.transform.Field "); } else { String visibility = JdtFlags.getVisibilityString(getVisibility()); - if (!visibility.equals("")) { + if (!"".equals(visibility)) { sb.append(visibility).append(' '); } } @@ -376,7 +380,7 @@ private String createFieldText(int indentLevel) throws BadLocationException, Mal private TextEditGroup declarationToReferenceTextEditGroup() { TextEditGroup group = new TextEditGroup("Convert local variable declaration to reference."); - if (!getContainingMethodNode().isScriptBody() || getContainingClosureExpression() != null) { + if ((getContainingMethodNode() != null && !getContainingMethodNode().isScriptBody()) || getContainingClosureExpression() != null) { int typeOrDefLength = variableExpressionInDeclaration.getStart() - getDeclarationOffset(); group.addTextEdit(new ReplaceEdit(getDeclarationOffset(), typeOrDefLength, "")); } @@ -385,7 +389,7 @@ private TextEditGroup declarationToReferenceTextEditGroup() { private TextEditGroup renameVariableReferencesTextEditGroup(RefactoringStatus status) { TextEditGroup group = new TextEditGroup("Update local variables to reference field."); - if (!getContainingMethodNode().isScriptBody() || getContainingClosureExpression() != null) { + if ((getContainingMethodNode() != null && !getContainingMethodNode().isScriptBody() || getContainingClosureExpression() != null)) { final Set references = new HashSet<>(); GroovyClassVisitor referencesVisitor = new DepthFirstVisitor() { @Override @@ -435,14 +439,13 @@ private ModuleNode getModuleNode() { private ClassNode getContainingClassNode() { if (containingClassNode == null) { - ModuleNode moduleNode = getModuleNode(); - if (moduleNode == null) { + if (getModuleNode() == null) { return null; } if (declarationExpression == null) { return null; } - containingClassNode = ASTTools.getContainingClassNode(moduleNode, variableExpressionInDeclaration.getStart()); + containingClassNode = ASTTools.getContainingClassNode(getModuleNode(), variableExpressionInDeclaration.getStart()); } return containingClassNode; } @@ -574,7 +577,6 @@ public void visitVariableExpression(VariableExpression expr) { try { visitor.visitModule(getModuleNode()); } catch (VisitCompleteException expected) { - ; } } }