Skip to content

Commit

Permalink
Merge pull request apache#7670 from lahodaj/rename-record-component
Browse files Browse the repository at this point in the history
Fixing rename refactoring for record components and compact constructors
  • Loading branch information
lahodaj authored Aug 30, 2024
2 parents 5c4660a + 854f904 commit b138eeb
Show file tree
Hide file tree
Showing 25 changed files with 1,469 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -268,16 +268,24 @@ private static Token<JavaTokenId> findIdentifierSpanImpl(CompilationInfo info, T

if (class2Kind.get(MethodTree.class).contains(leaf.getKind())) {
MethodTree method = (MethodTree) leaf;
TreePath parentPath = decl.getParentPath();
List<Tree> rightTrees = new ArrayList<Tree>();

rightTrees.addAll(method.getParameters());
boolean ignoreParameters = parentPath.getLeaf().getKind() == Kind.RECORD &&
!method.getParameters().isEmpty() &&
info.getTreeUtilities().isSynthetic(new TreePath(decl, method.getParameters().get(0)));

if (!ignoreParameters) {
rightTrees.addAll(method.getParameters());
}

rightTrees.addAll(method.getThrows());
rightTrees.add(method.getBody());

Name name = method.getName();

if (method.getReturnType() == null)
name = ((ClassTree) decl.getParentPath().getLeaf()).getSimpleName();
name = ((ClassTree) parentPath.getLeaf()).getSimpleName();

return findIdentifierSpanImpl(info, leaf, method.getReturnType(), rightTrees, name.toString(), info.getCompilationUnit(), info.getTrees().getSourcePositions());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,36 @@ public void testMatchBindings() throws Exception {
"[MARK_OCCURRENCES], 3:30-3:33");
}

public void testRecordCompactConstructors1() throws Exception {
performTest("MatchBindings.java",
"public record MatchBindings(String compact) {\n" +
" public MatchBindings {\n" +
" }\n" +
" private static MatchBindings create() {\n" +
" return new MatchBindings(null);\n" +
" }\n" +
"}\n",
1,
20,
"[MARK_OCCURRENCES], 1:11-1:24",
"[MARK_OCCURRENCES], 4:19-4:32");
}

public void testRecordCompactConstructors2() throws Exception {
performTest("MatchBindings.java",
"public record MatchBindings(String compact) {\n" +
" public MatchBindings {\n" +
" }\n" +
" private static MatchBindings create() {\n" +
" return new MatchBindings(null);\n" +
" }\n" +
"}\n",
4,
20,
"[MARK_OCCURRENCES], 1:11-1:24",
"[MARK_OCCURRENCES], 4:19-4:32");
}

//Support for exotic identifiers has been removed 6999438
public void REMOVEDtestExoticIdentifiers1() throws Exception {
performTest("ExoticIdentifier", 3, 43);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,9 @@ private static boolean allowInstantRename(CompilationInfo info, Element e, Eleme
return false;
}
}
if (info.getElementUtilities().getLinkedRecordElements(e).size() > 1) {
return false;
}
if (org.netbeans.modules.java.editor.base.semantic.Utilities.isPrivateElement(e)) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,16 @@ public void testIsInaccessibleOutsideOuterClassForAnnTypeMethodOfPrivateNestedCl
validateChangePoints(changePoints, 87, 93);
}

public void testNoInstanceRenameForRecordComponents() throws Exception {
sourceLevel = "17";

boolean[] wasResolved = new boolean[1];
Collection<Token> changePoints = performTest("package test; public class Test { private record Rec(String component) { } }", 120 - 55, wasResolved);

assertNull(changePoints);
assertTrue(wasResolved[0]);
}

private void validateChangePoints(Collection<Token> changePoints, int... origs) {
Set<Pair> awaited = new HashSet<Pair>();

Expand Down Expand Up @@ -345,6 +355,7 @@ public String toString() {
}

private FileObject source;
private String sourceLevel;

private Collection<Token> performTest(String sourceCode, final int offset, boolean[] wasResolved) throws Exception {
FileObject root = makeScratchDir(this);
Expand All @@ -358,7 +369,11 @@ private Collection<Token> performTest(String sourceCode, final int offset, boole
writeIntoFile(source, sourceCode);

SourceUtilsTestUtil.prepareTest(sourceDir, buildDir, cacheDir, new FileObject[0]);


if (sourceLevel != null) {
SourceUtilsTestUtil.setSourceLevel(sourceDir, sourceLevel);
}

DataObject od = DataObject.find(source);
EditorCookie ec = od.getCookie(EditorCookie.class);
Document doc = ec.openDocument();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@
* @author lahvac
*/
public class InstantRenamePerformerTest extends NbTestCase {


private String sourceLevel;

public InstantRenamePerformerTest(String testName) {
super(testName);
}
Expand Down Expand Up @@ -193,6 +195,13 @@ public void testPatternBinding() throws Exception {
performTest("package test; public class Test { public void test(Object o) {boolean b = o instanceof String s|tr && str.isEmpty(); } }", 117 - 22, ke, "package test; public class Test { public void test(Object o) {boolean b = o instanceof String satr && satr.isEmpty(); } }", true);
}

public void testRecordWithCompactConstructor1() throws Exception {
sourceLevel = "17";

KeyEvent ke = new KeyEvent(new JFrame(), KeyEvent.KEY_TYPED, 0, 0, KeyEvent.VK_UNDEFINED, 'e');
performTest("package test; public class Test { private record R|c(String str) { public Rc {} } }", 72 - 22, ke, - 1, "package test; public class Test { private record Rec(String str) { public Rec {} } }", true);
}

private void performTest(String sourceCode, int offset, KeyEvent ke, String golden, boolean stillInRename) throws Exception {
performTest(sourceCode, offset, ke, -1, golden, stillInRename);
}
Expand All @@ -217,6 +226,9 @@ private void performTest(String sourceCode, int offset, KeyEvent[] kes, int sele
TestUtilities.copyStringToFile(source, sourceCode.replaceFirst(Pattern.quote("|"), ""));

SourceUtilsTestUtil.prepareTest(sourceDir, buildDir, cacheDir, new FileObject[0]);
if (sourceLevel != null) {
SourceUtilsTestUtil.setSourceLevel(sourceDir, sourceLevel);
}
SourceUtilsTestUtil.compileRecursively(sourceDir);

DataObject od = DataObject.find(source);
Expand Down
12 changes: 12 additions & 0 deletions java/java.source.base/apichanges.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@
<apidef name="javasource_base">Java Source API</apidef>
</apidefs>
<changes>
<change id="TreeMaker.RecordComponent">
<api name="javasource_base" />
<summary>Adding TreeMaker.RecordComponent</summary>
<version major="1" minor="2.70.0"/>
<date day="20" month="8" year="2023"/>
<author login="jlahoda"/>
<compatibility addition="yes" binary="compatible" source="compatible"/>
<description>
Adding TreeMaker.RecordComponent and ElementUtilities.getLinkedRecordElements.
</description>
<class name="TreeMaker" package="org.netbeans.api.java.source"/>
</change>
<change id="TreeMaker.StringTemplate">
<api name="javasource_base" />
<summary>Adding TreeMaker.StringTemplate</summary>
Expand Down
2 changes: 1 addition & 1 deletion java/java.source.base/nbproject/project.properties
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ javadoc.name=Java Source Base
javadoc.title=Java Source Base
javadoc.arch=${basedir}/arch.xml
javadoc.apichanges=${basedir}/apichanges.xml
spec.version.base=2.69.0
spec.version.base=2.70.0
test.qa-functional.cp.extra=${refactoring.java.dir}/modules/ext/nb-javac-api.jar
test.unit.run.cp.extra=${o.n.core.dir}/core/core.jar:\
${o.n.core.dir}/lib/boot.jar:\
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ private T resolveImpl (final ModuleElement module, final JavacTaskImpl jt) {
}
case PARAMETER:
{
assert signatures.length == 3;
assert signatures.length == 4;
final Element type = getTypeElementByBinaryName (module, signatures[0], jt);
if (type instanceof TypeElement) {
final List<? extends Element> members = type.getEnclosedElements();
Expand Down Expand Up @@ -526,7 +526,12 @@ public static ElementHandle<ModuleElement> createModuleElementHandle(
ElementKind eek = ee.getKind();
if (eek == ElementKind.METHOD || eek == ElementKind.CONSTRUCTOR) {
assert ee instanceof ExecutableElement;
String[] _sigs = ClassFileUtil.createExecutableDescriptor((ExecutableElement)ee);
ExecutableElement eel = (ExecutableElement)ee;
if (!eel.getParameters().contains(element)) {
//may be e.g. a lambda parameter:
throw new IllegalArgumentException("Not a parameter for a method or a constructor.");
}
String[] _sigs = ClassFileUtil.createExecutableDescriptor(eel);
signatures = new String[_sigs.length + 1];
System.arraycopy(_sigs, 0, signatures, 0, _sigs.length);
signatures[_sigs.length] = element.getSimpleName().toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;

import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
Expand Down Expand Up @@ -928,7 +929,143 @@ public ExecutableElement getDescriptorElement(TypeElement origin) {
}
return null;
}


/**
* Find all elements that are linked record elements for the given input. Will
* return the record component element, field, accessor method, and canonical constructor
* parameters.
*
* This method can be called on any {@code Element}, and will return a collection
* with a single entry if the provided element is not a record element.
*
* @param forElement for which the linked elements should be found
* @return a collection containing the provided element, plus any additional elements
* that are linked to it by the Java record specification
* @since 2.70
*/
public Collection<? extends Element> getLinkedRecordElements(Element forElement) {
Parameters.notNull("forElement", forElement);

TypeElement record = null;
Name componentName = null;

switch (forElement.getKind()) {
case FIELD -> {
Element enclosing = forElement.getEnclosingElement();
if (enclosing.getKind() == ElementKind.RECORD) {
record = (TypeElement) enclosing;
componentName = forElement.getSimpleName();
}
}
case PARAMETER -> {
Element enclosing = forElement.getEnclosingElement();
if (enclosing.getKind() == ElementKind.CONSTRUCTOR) {
Element enclosingType = enclosing.getEnclosingElement();
if (enclosingType.getKind() == ElementKind.RECORD) {
TypeElement recordType = (TypeElement) enclosingType;
ExecutableElement constructor = recordCanonicalConstructor(recordType);
if (constructor != null && constructor.equals(enclosing)) {
int idx = constructor.getParameters().indexOf(forElement);
if (idx >= 0 && idx < recordType.getRecordComponents().size()) {
RecordComponentElement component = recordType.getRecordComponents().get(idx);
if (component.getSimpleName().equals(forElement.getSimpleName())) {
record = recordType;
componentName = component.getSimpleName();
}
}
}
}
}
}
case METHOD -> {
Element enclosing = forElement.getEnclosingElement();
ExecutableElement method = (ExecutableElement) forElement;
if (method.getParameters().isEmpty() && enclosing.getKind() == ElementKind.RECORD) {
TypeElement recordType = (TypeElement) enclosing;
for (RecordComponentElement component : recordType.getRecordComponents()) {
if (forElement.equals(component.getAccessor())) {
record = recordType;
componentName = component.getSimpleName();
}
}
}
}
case RECORD_COMPONENT -> {
record = (TypeElement) forElement.getEnclosingElement();
componentName = forElement.getSimpleName();
}
}

if (record == null) {
return Collections.singleton(forElement);
}

RecordComponentElement component = null;
int componentIdx = 0;

for (RecordComponentElement c : record.getRecordComponents()) {
if (c.getSimpleName().equals(componentName)) {
component = c;
break;
}
componentIdx++;
}

if (component == null) {
//erroneous state(?), ignore:
return Collections.singleton(forElement);
}

Set<Element> result = new HashSet<>();

result.add(component);
result.add(component.getAccessor());

for (Element el : record.getEnclosedElements()) {
if (el.getKind() == ElementKind.FIELD && el.getSimpleName().equals(componentName)) {
result.add(el);
break;
}
}

ExecutableElement canonicalConstructor = recordCanonicalConstructor(record);
if (canonicalConstructor != null && componentIdx < canonicalConstructor.getParameters().size()) {
result.add(canonicalConstructor.getParameters().get(componentIdx));
}

return result;
}

private ExecutableElement recordCanonicalConstructor(TypeElement recordType) {
Supplier<ExecutableElement> fallback =
() -> {
List<? extends RecordComponentElement> recordComponents = recordType.getRecordComponents();
for (ExecutableElement c : ElementFilter.constructorsIn(recordType.getEnclosedElements())) {
if (recordComponents.size() == c.getParameters().size()) {
Iterator<? extends RecordComponentElement> componentIt = recordComponents.iterator();
Iterator<? extends VariableElement> parameterIt = c.getParameters().iterator();
boolean componentMatches = true;

while (componentIt.hasNext() && parameterIt.hasNext() && componentMatches) {
TypeMirror componentType = componentIt.next().asType();
TypeMirror parameterType = parameterIt.next().asType();

componentMatches &= info.getTypes().isSameType(componentType, parameterType);
}
if (componentMatches) {
return c;
}
}
}
return null;
};
return ElementFilter.constructorsIn(recordType.getEnclosedElements())
.stream()
.filter(info.getElements()::isCanonicalConstructor)
.findAny()
.orElseGet(fallback);
}

// private implementation --------------------------------------------------

private static final Set<Modifier> NOT_OVERRIDABLE = EnumSet.of(Modifier.STATIC, Modifier.FINAL, Modifier.PRIVATE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,21 @@ public VariableTree Variable(ModifiersTree modifiers,
return delegate.Variable(modifiers, name, type, initializer);
}

/**
* Creates a new VariableTree for a record component.
*
* @param modifiers the modifiers of this record component.
* @param name the name of the record component.
* @param type the type of this record component.
* @see com.sun.source.tree.VariableTree
* @since 2.70
*/
public VariableTree RecordComponent(ModifiersTree modifiers,
CharSequence name,
Tree type) {
return delegate.RecordComponent(modifiers, name, type);
}

/**
* Creates a new BindingPatternTree.
* @deprecated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@

import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.Name;
import javax.lang.model.element.TypeElement;
Expand Down Expand Up @@ -322,6 +323,14 @@ private static boolean isSupported(Element el) {
case RECORD:
//TODO: record component
return true;
case PARAMETER:
//only method and constructor parameters supported (not lambda):
if (el.getEnclosingElement().getKind() == ElementKind.METHOD ||
el.getEnclosingElement().getKind() == ElementKind.CONSTRUCTOR) {
return ((ExecutableElement) el.getEnclosingElement()).getParameters().contains(el);
} else {
return false;
}
default:
return false;
}
Expand Down
Loading

0 comments on commit b138eeb

Please sign in to comment.