Skip to content

Commit

Permalink
[#1984] improve auto-generated paramLabel in Kotlin
Browse files Browse the repository at this point in the history 
Kotlin `internal` methods have mangled names with embedded "$"; truncate this portion off the paramLabel
  • Loading branch information
@remkop
remkop committed Apr 2, 2023
1 parent bd65779 commit 50a3a6d
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 13 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Artifacts in this release are signed by Remko Popma (6601 E5C0 8DCC BB96).
* [#1959] API: Add ability to enable loading resource bundles in annotation processor for tests.
* [#1975][#1976] Enhancement: Fixed `isJansiConsoleInstalled` performance issue. Thanks to [ChrisTrenkamp](https://github.com/ChrisTrenkamp) for the pull request.
* [#1932] Enhancement: Move System-Rules tests to Java 5 test module; move System-Lambda tests to Java 8+ test module. Facilitate testing with recent JRE's.
* [#1984] Enhancement (Kotlin): improve `paramLabel` string auto-generated from Kotlin `internal` methods which have mangled names with embedded "$". Thanks to [Ken Yee](https://github.com/kenyee) for raising this.
* [#1945] DOC: Code sample: add Java version. Thanks to [Andreas Deininger](https://github.com/deining) for the pull request.
* [#1956] Doc: Fix broken link in user manual. Thanks to [Andreas Deininger](https://github.com/deining) for the pull request.
* [#1955] DEP: Bump asciidoctorj from 2.5.5 to 2.5.7. Thanks to [Andreas Deininger](https://github.com/deining) for the pull request.
Expand Down
20 changes: 13 additions & 7 deletions src/main/java/picocli/CommandLine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9547,7 +9547,7 @@ abstract static class Builder<T extends Builder<T>> {
arity = Range.optionArity(annotatedElement);
required = option.required();

paramLabel = inferLabel(option.paramLabel(), annotatedElement.getName(), annotatedElement.getTypeInfo());
paramLabel = inferLabel(option.paramLabel(), annotatedElement);

hideParamSyntax = option.hideParamSyntax();
interactive = option.interactive();
Expand Down Expand Up @@ -9585,9 +9585,9 @@ abstract static class Builder<T extends Builder<T>> {

// method parameters may be positional parameters without @Parameters annotation
if (parameters == null) {
paramLabel = inferLabel(null, annotatedElement.getName(), annotatedElement.getTypeInfo());
paramLabel = inferLabel(null, annotatedElement);
} else {
paramLabel = inferLabel(parameters.paramLabel(), annotatedElement.getName(), annotatedElement.getTypeInfo());
paramLabel = inferLabel(parameters.paramLabel(), annotatedElement);

hideParamSyntax = parameters.hideParamSyntax();
interactive = parameters.interactive();
Expand Down Expand Up @@ -9619,14 +9619,20 @@ abstract static class Builder<T extends Builder<T>> {
}
}
}
private static String inferLabel(String label, String fieldName, ITypeInfo typeInfo) {
private static String inferLabel(String label, IAnnotatedElement annotatedElement) {
if (!empty(label)) { return label.trim(); }
String name = fieldName;
if (typeInfo.isMap()) { // #195 better param labels for map fields
List<ITypeInfo> aux = typeInfo.getAuxiliaryTypeInfos();
String name = annotatedElement.getName();
if (annotatedElement.getTypeInfo().isMap()) { // #195 better param labels for map fields
List<ITypeInfo> aux = annotatedElement.getTypeInfo().getAuxiliaryTypeInfos();
if (aux.size() < 2 || aux.get(0) == null || aux.get(1) == null) {
name = "String=String";
} else { name = aux.get(0).getClassSimpleName() + "=" + aux.get(1).getClassSimpleName(); }
} else {
int dollar = name.indexOf('$');
if (dollar >= 0 && Method.class.equals(annotatedElement.userObject().getClass())) {
// #1984 Kotlin internal members have mangled names with "$" followed by random postfix
name = name.substring(0, dollar);
}
}
return "<" + name + ">";
}
Expand Down
57 changes: 51 additions & 6 deletions src/test/java/picocli/ModelArgSpecTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import picocli.CommandLine.Model.PositionalParamSpec;
import picocli.CommandLine.Model.RuntimeTypeInfo;

import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.ArrayList;
Expand Down Expand Up @@ -223,15 +224,59 @@ public void testArgSpecBuilderCompletionCandidates() {
assertEquals(candidates, positional.completionCandidates());
}

private static class AnnotatedImpl implements CommandLine.Model.IAnnotatedElement {
Object userObject;
String name;
ITypeInfo typeInfo;

AnnotatedImpl() {}
AnnotatedImpl(String name, ITypeInfo typeInfo) {
this(null, name, typeInfo);
}
AnnotatedImpl(Object userObject, String name, ITypeInfo typeInfo) {
this.userObject = userObject;
this.name = name;
this.typeInfo = typeInfo;
}

public Object userObject() { return userObject; }
public boolean isAnnotationPresent(Class<? extends Annotation> annotationClass) { return false;}
public <T extends Annotation> T getAnnotation(Class<T> annotationClass) { return null;}
public String getName() {return name;}
public String getMixinName() {return null;}
public boolean isArgSpec() {return false;}
public boolean isOption() {return false;}
public boolean isParameter() {return false;}
public boolean isArgGroup() {return false;}
public boolean isMixin() {return false;}
public boolean isUnmatched() { return false;}
public boolean isSpec() {return false;}
public boolean isParentCommand() {return false;}
public boolean isMultiValue() {return false;}
public boolean isInteractive() {return false;}
public boolean hasInitialValue() {return false;}
public boolean isMethodParameter() {return false;}
public int getMethodParamPosition() {return 0;}
public IScope scope() {return null;}
public IGetter getter() {return null;}
public ISetter setter() {return null;}
public ITypeInfo getTypeInfo() {return typeInfo;}
public String getToString() {return null;}
}

@Test
public void testArgSpecBuilderInferLabel() throws Exception{
Method m = ArgSpec.Builder.class.getDeclaredMethod("inferLabel", String.class, String.class, ITypeInfo.class);
Method m = ArgSpec.Builder.class.getDeclaredMethod("inferLabel", String.class, CommandLine.Model.IAnnotatedElement.class);
m.setAccessible(true);
assertEquals("<String=String>", m.invoke(null, "", "fieldName", typeInfo(new Class[0])));
assertEquals("<String=String>", m.invoke(null, "", "fieldName", typeInfo(new Class[]{Integer.class})));
assertEquals("<String=String>", m.invoke(null, "", "fieldName", typeInfo(new Class[]{null, Integer.class})));
assertEquals("<String=String>", m.invoke(null, "", "fieldName", typeInfo(new Class[]{Integer.class, null})));
assertEquals("<Integer=Integer>", m.invoke(null, "", "fieldName", typeInfo(new Class[]{Integer.class, Integer.class})));

assertEquals("<String=String>", m.invoke(null, "", new AnnotatedImpl("fieldName", typeInfo(new Class[0]))));
assertEquals("<String=String>", m.invoke(null, "", new AnnotatedImpl("fieldName", typeInfo(new Class[]{Integer.class}))));
assertEquals("<String=String>", m.invoke(null, "", new AnnotatedImpl("fieldName", typeInfo(new Class[]{null, Integer.class}))));
assertEquals("<String=String>", m.invoke(null, "", new AnnotatedImpl("fieldName", typeInfo(new Class[]{Integer.class, null}))));
assertEquals("<Integer=Integer>", m.invoke(null, "", new AnnotatedImpl("fieldName", typeInfo(new Class[]{Integer.class, Integer.class}))));

assertEquals("<prefix>", m.invoke(null, null, new AnnotatedImpl(m,"prefix$postfix", new TypeInfoAdapter())));

}

private ITypeInfo typeInfo(final Class<?>[] aux) {
Expand Down

0 comments on commit 50a3a6d

Please sign in to comment.