Skip to content

Commit

Permalink
Fix for #1308: add delegate or owner qualifier within closure/lambda
Browse files Browse the repository at this point in the history 
see #1288
  • Loading branch information
@eric-milles
eric-milles committed Dec 30, 2022
1 parent dc9d5fc commit af40b9d
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 15 deletions.
30 changes: 29 additions & 1 deletion ...se.tests/src/org/codehaus/groovy/eclipse/test/actions/ConvertToPropertyActionTests.groovy
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2021 the original author or authors.
* Copyright 2009-2022 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.
Expand Down Expand Up @@ -149,6 +149,20 @@ final class ConvertToPropertyActionTests extends GroovyEditorTestSuite {
assertEditorContents 'def getX(){0}\n[x:1].with { this.x }'
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1308
void testImplicitGetterToProperty3() {
def pogo = 'class C {\n def getX(){0}\n}\n'
convertToProperty pogo + "new C().with { get${CARET}X() }"
assertEditorContents pogo + 'new C().with { x }' // no qualifier

convertToProperty pogo + "def x=1\nnew C().with { get${CARET}X() }"
assertEditorContents pogo + 'def x=1\nnew C().with { delegate.x }'

def stc = { '@groovy.transform.TypeChecked void test(){\n' + it + '\n}' }
convertToProperty pogo + stc("def x=1\nnew C().with { get${CARET}X() }")
assertEditorContents pogo + stc('def x=1\nnew C().with { delegate.x }')
}

@Test
void testImplicitIsserToProperty() {
addGroovySource 'class Foo { static void isSomething() {} }', 'Foo'
Expand All @@ -168,6 +182,20 @@ final class ConvertToPropertyActionTests extends GroovyEditorTestSuite {
assertEditorContents 'void setX(x){}\n[x:1].with { this.x = 0 }'
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1308
void testImplicitSetterToProperty3() {
def pogo = 'class C {\n void setX(x){}\n}\n'
convertToProperty pogo + "new C().with { set${CARET}X(0) }"
assertEditorContents pogo + 'new C().with { x = 0 }' // no qualifier

convertToProperty pogo + "def x=1\nnew C().with { set${CARET}X(0) }"
assertEditorContents pogo + 'def x=1\nnew C().with { delegate.x = 0 }'

def stc = { '@groovy.transform.TypeChecked void test(){\n' + it + '\n}' }
convertToProperty pogo + stc("def x=1\nnew C().with { set${CARET}X(0) }")
assertEditorContents pogo + stc('def x=1\nnew C().with { delegate.x = 0 }')
}

@Test
void testStaticGetterToProperty() {
convertToProperty "import java.lang.management.*; ManagementFactory.getRun${CARET}timeMXBean()"
Expand Down
42 changes: 28 additions & 14 deletions ...lipse.ui/src/org/codehaus/groovy/eclipse/refactoring/actions/ConvertToPropertyAction.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2021 the original author or authors.
* Copyright 2009-2022 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.
Expand All @@ -18,6 +18,7 @@
import static java.beans.Introspector.decapitalize;
import static java.util.regex.Pattern.compile;

import static org.codehaus.groovy.transform.stc.StaticTypesMarker.IMPLICIT_RECEIVER;
import static org.eclipse.jdt.core.formatter.DefaultCodeFormatterConstants.FORMATTER_INSERT_SPACE_AFTER_ASSIGNMENT_OPERATOR;
import static org.eclipse.jdt.core.formatter.DefaultCodeFormatterConstants.FORMATTER_INSERT_SPACE_BEFORE_ASSIGNMENT_OPERATOR;

Expand Down Expand Up @@ -84,12 +85,12 @@ public void run() {
}
}

public static TextEdit createEdit(final GroovyCompilationUnit gcu, final int pos, final int len) {
public static TextEdit createEdit(final GroovyCompilationUnit gcu, final int idx, final int len) {
ModuleNodeInfo info = gcu.getModuleInfo(true);
if (!info.isEmpty()) {
MethodCall call = null;

ASTNode node = new ASTNodeFinder(new Region(pos, len)).doVisit(info.module);
ASTNode node = new ASTNodeFinder(new Region(idx, len)).doVisit(info.module);
if (node instanceof ConstantExpression) {
IASTFragment fragment = new FindSurroundingNode(new Region(node)).doVisitSurroundingNode(info.module);
if (fragment.kind() == ASTFragmentKind.METHOD_CALL) {
Expand All @@ -114,8 +115,8 @@ public static TextEdit createEdit(final GroovyCompilationUnit gcu, final int pos
TextEdit edit = new ReplaceEdit(offset, length, decapitalize(propertyName));

// implicit-this call may require qualifier to retain its semantics
if (call.getReceiver().getEnd() < 1 && isTypeChange(gcu, edit, node)) {
edit = new ReplaceEdit(offset, length, call.getReceiver().getText() + "." + decapitalize(propertyName));
if (call.getReceiver().getEnd() < 1 && isTypeChange(edit, node, gcu)) {
edit = new ReplaceEdit(offset, length, getReceiver(call) + "." + decapitalize(propertyName));
}

return edit;
Expand Down Expand Up @@ -149,9 +150,10 @@ public static TextEdit createEdit(final GroovyCompilationUnit gcu, final int pos
}

// implicit-this call may require qualifier to retain its semantics
if (call.getReceiver().getEnd() < 1 && isTypeChange(gcu, edit, node)) {
edit.removeChild(0); // add qualifier to the property name
replacement.insert(0, call.getReceiver().getText() + ".");
if (call.getReceiver().getEnd() < 1 && isTypeChange(edit, node, gcu)) {
edit.removeChild(0);
// add qualifier to the property name
replacement.insert(0, getReceiver(call) + ".");
edit.addChild(new ReplaceEdit(offset, length, replacement.toString()));
}

Expand All @@ -164,14 +166,26 @@ public static TextEdit createEdit(final GroovyCompilationUnit gcu, final int pos

//--------------------------------------------------------------------------

private static boolean isTypeChange(final GroovyCompilationUnit gcu, final TextEdit edit, final ASTNode node) {
private static String getReceiver(final MethodCall call) {
String receiver = ((ASTNode) call).getNodeMetaData(IMPLICIT_RECEIVER);
if (receiver == null) {
receiver = call.getReceiver().getText();
}
return receiver;
}

private static boolean isTypeChange(final TextEdit edit, final ASTNode node, final GroovyCompilationUnit unit) {
try {
TypeLookupResult before = inferNodeType(node, gcu);
TextEdit undo = gcu.applyTextEdit(edit.copy(), null);
TypeLookupResult after = inferNodeType(new ASTNodeFinder(new Region(node.getStart(), 0)).doVisit(gcu.getModuleNode()), gcu);
gcu.applyTextEdit(undo, null);
TypeLookupResult before = lookupNodeType(node, unit);
TextEdit undo = unit.applyTextEdit(edit.copy(), null);
TypeLookupResult after = lookupNodeType(new ASTNodeFinder(new Region(node.getStart(), 0)).doVisit(unit.getModuleNode()), unit);
unit.applyTextEdit(undo, null);

if (!before.declaringType.equals(after.declaringType)) {
if (!before.declaringType.equals(before.scope.getThis())) {
ASTNode call = (node instanceof MethodCall ? node : before.scope.getEnclosingNode()); // TODO: refactor side-effect solution!
call.getNodeMetaData(IMPLICIT_RECEIVER, x -> before.declaringType.equals(before.scope.getDelegate()) ? "delegate" : "owner");
}
return true;
}
} catch (Exception e) {
Expand All @@ -180,7 +194,7 @@ private static boolean isTypeChange(final GroovyCompilationUnit gcu, final TextE
return false;
}

private static TypeLookupResult inferNodeType(final ASTNode node, final GroovyCompilationUnit unit) {
private static TypeLookupResult lookupNodeType(/* */ final ASTNode node, final GroovyCompilationUnit unit) {
TypeLookupResult[] result = new TypeLookupResult[1];
new TypeInferencingVisitorFactory().createVisitor(unit).visitCompilationUnit((n, r, x) -> {
if (n == node) {
Expand Down

0 comments on commit af40b9d

Please sign in to comment.