Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.0] fix parameters handling #402

Merged
merged 5 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file removed .github/release/maven-settings.xml.gpg
Binary file not shown.
Binary file removed .github/release/smallrye-sign.asc.gpg
Binary file not shown.
3 changes: 1 addition & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ jobs:
- 8
- 11
- 17
- 19
- 20-ea
- 21
compiler:
- javac
- ecj
Expand Down
16 changes: 11 additions & 5 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,30 @@ jobs:
with:
distribution: temurin
java-version: 17
server-id: 'oss.sonatype'
server-username: 'MAVEN_DEPLOY_USERNAME'
server-password: 'MAVEN_DEPLOY_TOKEN'
gpg-private-key: ${{secrets.MAVEN_GPG_PRIVATE_KEY}}
gpg-passphrase: 'MAVEN_GPG_PASSPHRASE'

- name: Maven release ${{steps.metadata.outputs.current-version}}
env:
MAVEN_DEPLOY_USERNAME: ${{secrets.MAVEN_DEPLOY_USERNAME}}
MAVEN_DEPLOY_TOKEN: ${{secrets.MAVEN_DEPLOY_TOKEN}}
MAVEN_GPG_PASSPHRASE: ${{secrets.MAVEN_GPG_PASSPHRASE}}
run: |
export MAVEN_OPTS="--add-opens=java.base/java.util=ALL-UNNAMED"
java -version
gpg --quiet --batch --yes --decrypt --passphrase="${{secrets.SECRET_PASSPHRASE}}" --output smallrye-sign.asc .github/release/smallrye-sign.asc.gpg
gpg --quiet --batch --yes --decrypt --passphrase="${{secrets.SECRET_PASSPHRASE}}" --output maven-settings.xml .github/release/maven-settings.xml.gpg
gpg --fast-import --no-tty --batch --yes smallrye-sign.asc
git config --global user.name "SmallRye CI"
git config --global user.email "smallrye@googlegroups.com"
git checkout -b release
sed -i -e 's|<version.jandex>.*</version.jandex>|<version.jandex>${{steps.metadata.outputs.current-version}}</version.jandex>|' benchmarks/pom.xml
sed -i -e 's|^version: main|version: ${{steps.metadata.outputs.current-version}}|' doc/antora.yml
git commit -a -m 'Amendments before release'
mvn -B release:prepare -Prelease -DreleaseVersion=${{steps.metadata.outputs.current-version}} -DdevelopmentVersion=${{steps.metadata.outputs.next-version}} -s maven-settings.xml
mvn -B release:prepare -Prelease -DreleaseVersion=${{steps.metadata.outputs.current-version}} -DdevelopmentVersion=${{steps.metadata.outputs.next-version}}
git checkout ${{github.base_ref}}
git rebase release
mvn -B release:perform -Prelease -s maven-settings.xml
mvn -B release:perform -Prelease
sed -i -e 's|<version.jandex>.*</version.jandex>|<version.jandex>${project.version}</version.jandex>|' benchmarks/pom.xml
sed -i -e 's|^version: ${{steps.metadata.outputs.current-version}}|version: main|' doc/antora.yml
git commit -a -m 'Amendments after release'
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jboss/jandex/AnnotationTarget.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public enum Kind {

/**
* An object of type {@link org.jboss.jandex.RecordComponentInfo}
*
*
* @since 2.4
*/
RECORD_COMPONENT
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/jboss/jandex/ClassInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ public final boolean hasAnnotation(DotName name) {
* instance may be used to determine the exact location of the annotation instance.
* <p>
* The following is a non-exhaustive list of examples of annotations returned by this method:
*
*
* <pre class="brush:java">
* {@literal @}MyClassAnnotation
* public class Foo&lt;{@literal @}MyTypeAnnotation T&gt; {
Expand Down Expand Up @@ -386,7 +386,7 @@ public final AnnotationInstance annotation(DotName name) {
* instances may be used to determine the exact location of the respective annotation instance.
* <p>
* The following is a non-exhaustive list of examples of annotations returned by this method:
*
*
* <pre class="brush:java">
* {@literal @}MyClassAnnotation
* public class Foo&lt;{@literal @}MyTypeAnnotation T&gt; {
Expand Down Expand Up @@ -469,7 +469,7 @@ public final List<AnnotationInstance> annotationsWithRepeatable(DotName name, In
* instances may be used to determine the exact location of the respective annotation instance.
* <p>
* The following is a non-exhaustive list of examples of annotations returned by this method:
*
*
* <pre class="brush:java">
* {@literal @}MyClassAnnotation
* public class Foo&lt;{@literal @}MyTypeAnnotation T&gt; {
Expand Down
8 changes: 4 additions & 4 deletions core/src/main/java/org/jboss/jandex/FieldInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public final boolean hasAnnotation(DotName name) {
* of the annotation instance.
* <p>
* The following is a non-exhaustive list of examples of annotations returned by this method:
*
*
* <pre class="brush:java">
* {@literal @}MyFieldAnnotation
* public String foo;
Expand All @@ -144,7 +144,7 @@ public final AnnotationInstance annotation(DotName name) {
* of the respective annotation instance.
* <p>
* The following is a non-exhaustive list of examples of annotations returned by this method:
*
*
* <pre class="brush:java">
* {@literal @}MyFieldAnnotation
* public String foo;
Expand Down Expand Up @@ -215,7 +215,7 @@ public final List<AnnotationInstance> annotationsWithRepeatable(DotName name, In
* of the respective annotation instance.
* <p>
* The following is a non-exhaustive list of examples of annotations returned by this method:
*
*
* <pre class="brush:java">
* {@literal @}MyFieldAnnotation
* public String foo;
Expand Down Expand Up @@ -356,7 +356,7 @@ public final short flags() {
}

/**
*
*
* @return {@code true} if this field is a synthetic field
*/
public final boolean isSynthetic() {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jboss/jandex/IndexView.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ default ClassInfo getClassByName(Class<?> clazz) {
* <p>
* Note that this will only pick up direct subclasses of the class. It will not
* pick up subclasses of subclasses.
*
*
* @param className the super class of the desired subclasses
* @return a non-null list of all known subclasses of className
*/
Expand Down
153 changes: 90 additions & 63 deletions core/src/main/java/org/jboss/jandex/Indexer.java
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,7 @@ private static void skipFully(InputStream s, long n) throws IOException {
private List<MethodInfo> methods;
private List<FieldInfo> fields;
private List<RecordComponentInfo> recordComponents;
private byte[][] debugParameterNames;
private byte[][] methodParameterNames;
private IdentityHashMap<MethodInfo, MethodParamList> methodParams;
private List<DotName> modulePackages;
private DotName moduleMainClass;

Expand Down Expand Up @@ -375,6 +374,7 @@ private void initClassFields() {

modulePackages = null;
moduleMainClass = null;
methodParams = new IdentityHashMap<>();
}

private void processMethodInfo(DataInputStream data) throws IOException {
Expand All @@ -397,16 +397,17 @@ private void processMethodInfo(DataInputStream data) throws IOException {
if (parameters.length == 0 && Arrays.equals(Utils.INIT_METHOD_NAME, name)) {
currentClass.setHasNoArgsConstructor(true);
}
methodParameterNames = debugParameterNames = null;
MethodParamList parameterList = new MethodParamList(method);
methodParams.put(method, parameterList);
processAttributes(data, method);
method.setAnnotations(elementAnnotations);
elementAnnotations.clear();
parameterList.finish();

// Prefer method parameter names over debug info
if (methodParameterNames != null)
method.methodInternal().setParameterNames(methodParameterNames);
else if (debugParameterNames != null)
method.methodInternal().setParameterNames(debugParameterNames);
byte[][] parameterNames = parameterList.getNames();
if (parameterNames != null) {
method.methodInternal().setParameterNames(parameterNames);
}

methods.add(method);
}
Expand Down Expand Up @@ -703,30 +704,20 @@ private void processMethodParameters(DataInputStream data, MethodInfo target) th
// the one we just read (for extra safety, do NOT do this for compliant methods)
numParameters = target.parametersCount();
}
byte[][] parameterNames = numParameters > 0 ? new byte[numParameters][] : MethodInternal.EMPTY_PARAMETER_NAMES;
int filledParameters = 0;
for (int i = 0; i < numParameters; i++) {
int nameIndex = data.readUnsignedShort();
byte[] parameterName = nameIndex == 0 ? null : decodeUtf8EntryAsBytes(nameIndex);
int flags = data.readUnsignedShort();
// skip synthetic/mandated params to get the same param name index as annotations (which do not count them)
if ((flags & (Modifiers.SYNTHETIC | Modifiers.MANDATED)) != 0) {
continue;
}
boolean syntheticOrMandated = (flags & (Modifiers.SYNTHETIC | Modifiers.MANDATED)) != 0;

parameterNames[filledParameters++] = parameterName;
if (methodParams.containsKey(target)) { // should always be there, just being extra careful
methodParams.get(target).appendProper(parameterName, syntheticOrMandated);
}
}

byte[][] realParameterNames = filledParameters > 0 ? new byte[filledParameters][]
: MethodInternal.EMPTY_PARAMETER_NAMES;
if (filledParameters > 0)
System.arraycopy(parameterNames, 0, realParameterNames, 0, filledParameters);
this.methodParameterNames = realParameterNames;
}

private void processLocalVariableTable(DataInputStream data, MethodInfo target) throws IOException {
int numVariables = data.readUnsignedShort();
byte[][] variableNames = numVariables > 0 ? new byte[numVariables][] : MethodInternal.EMPTY_PARAMETER_NAMES;
int numParameters = 0;
for (int i = 0; i < numVariables; i++) {
int startPc = data.readUnsignedShort();
Expand All @@ -736,32 +727,38 @@ private void processLocalVariableTable(DataInputStream data, MethodInfo target)
int index = data.readUnsignedShort();

// parameters have startPc == 0
if (startPc != 0)
if (startPc != 0) {
continue;
}

byte[] parameterName = nameIndex == 0 ? null : decodeUtf8EntryAsBytes(nameIndex);

// ignore "this"
if (numParameters == 0 && parameterName != null && parameterName.length == 4
&& parameterName[0] == 0x74
&& parameterName[1] == 0x68
&& parameterName[2] == 0x69
&& parameterName[3] == 0x73)
&& parameterName[3] == 0x73) {
continue;
// ignore "this$*" that javac adds (not ECJ)
}

// treat "this$*" that javac adds (not ecj) as synthetic (or mandated)
boolean synthetic = false;
if (numParameters == 0 && parameterName != null && parameterName.length > 5
&& parameterName[0] == 0x74
&& parameterName[1] == 0x68
&& parameterName[2] == 0x69
&& parameterName[3] == 0x73
&& parameterName[4] == 0x24)
continue;
&& parameterName[4] == 0x24) {
synthetic = true;
}

// here we rely on the parameters being in the right order
variableNames[numParameters++] = parameterName;
if (methodParams.containsKey(target)) { // should always be there, just being extra careful
methodParams.get(target).appendDebug(parameterName, synthetic);
}

numParameters++;
}
byte[][] parameterNames = numParameters > 0 ? new byte[numParameters][] : MethodInternal.EMPTY_PARAMETER_NAMES;
if (numParameters > 0)
System.arraycopy(variableNames, 0, parameterNames, 0, numParameters);
this.debugParameterNames = parameterNames;
}

private void processEnclosingMethod(DataInputStream data, ClassInfo target) throws IOException {
Expand Down Expand Up @@ -905,9 +902,14 @@ private void adjustMethodParameters() {
}
alreadyProcessedMethods.put(method, null);

if (isInnerConstructor(method) && !signaturePresent.containsKey(method)) {
// inner class constructor without signature, list of parameter types is derived
// from the descriptor, which includes 1 mandated or synthetic parameter at the beginning
if (signaturePresent.containsKey(method)) {
// if the method has a signature, we derived the parameter list from it,
// and it never includes mandated or synthetic parameters
continue;
}

if (isInnerConstructor(method)) {
// inner class constructor, descriptor includes 1 mandated or synthetic parameter at the beginning
DotName enclosingClass = null;
if (method.declaringClass().enclosingClass() != null) {
enclosingClass = method.declaringClass().enclosingClass();
Expand All @@ -921,10 +923,8 @@ private void adjustMethodParameters() {
System.arraycopy(parameterTypes, 1, newParameterTypes, 0, parameterTypes.length - 1);
method.setParameters(intern(newParameterTypes));
}
} else if (isEnumConstructor(method) && !signaturePresent.containsKey(method)) {
// enum constructor without signature, list of parameter types is derived from the descriptor,
// which includes 2 synthetic parameters at the beginning -- let's remove those
// (javac typically emits the signature, while ecj does not)
} else if (isEnumConstructor(method)) {
// enum constructor, descriptor includes 2 synthetic parameters at the beginning
Type[] parameterTypes = method.methodInternal().parameterTypesArray();
if (parameterTypes.length >= 2
&& parameterTypes[0].kind() == Type.Kind.CLASS
Expand All @@ -944,6 +944,48 @@ private void adjustMethodParameters() {
}
}

private boolean isInnerConstructor(MethodInfo method) {
if (!method.isConstructor()) {
return false;
}

ClassInfo klass = method.declaringClass();

// there's no indication in the class file whether a local or anonymous class was declared in static context,
// so we use some heuristics here
if (klass.nestingType() == ClassInfo.NestingType.LOCAL
|| klass.nestingType() == ClassInfo.NestingType.ANONYMOUS) {

// synthetic or mandated parameter at the beginning of the parameter list is the enclosing instance,
// the constructor belongs to a "non-static" class
//
// this works for:
// - javac with `-g` or `-parameters`
// - javac since JDK 21 (see https://bugs.openjdk.org/browse/JDK-8292275)
// - ecj with `-parameters` (see also `processLocalVariableTable`)
MethodParamList parameters = methodParams.get(method);
if (parameters != null && parameters.firstIsEnclosingInstance()) {
return true;
}

// synthetic field named `this$*` is the enclosing instance,
// the constructor belongs to a "non-static" class
//
// this works for:
// - javac until JDK 18 (see https://bugs.openjdk.org/browse/JDK-8271623)
// - ecj
for (FieldInfo field : fields) {
if (Modifiers.isSynthetic(field.flags()) && field.name().startsWith("this$")) {
return true;
}
}

return false;
}

return klass.nestingType() == ClassInfo.NestingType.INNER && !Modifier.isStatic(klass.flags());
}

private static boolean isEnumConstructor(MethodInfo method) {
return method.declaringClass().isEnum() && method.isConstructor();
}
Expand Down Expand Up @@ -1021,29 +1063,6 @@ private static void setTypeParameters(AnnotationTarget target, Type[] typeParame

}

private boolean isInnerConstructor(MethodInfo method) {
if (!method.isConstructor()) {
return false;
}

ClassInfo klass = method.declaringClass();

// there's no indication in the class file whether a local or anonymous class was declared in static context,
// so we use a heuristic: a class was not declared in static context if it has a synthetic field named `this$N`
// (this seems to work both for javac and ecj)
if (klass.nestingType() == ClassInfo.NestingType.LOCAL
|| klass.nestingType() == ClassInfo.NestingType.ANONYMOUS) {
for (FieldInfo field : fields) {
if (Modifiers.isSynthetic(field.flags()) && field.name().startsWith("this$")) {
return true;
}
}
return false;
}

return klass.nestingType() == ClassInfo.NestingType.INNER && !Modifier.isStatic(klass.flags());
}

private void resolveTypeAnnotation(AnnotationTarget target, TypeAnnotationState typeAnnotationState) {
// Signature is erroneously omitted from bridge methods with generic type annotations
if (typeAnnotationState.genericsRequired && !signaturePresent.containsKey(target)) {
Expand Down Expand Up @@ -2385,6 +2404,14 @@ public ClassSummary indexWithSummary(InputStream stream) throws IOException {
signatures = null;
classSignatureIndex = -1;
signaturePresent = null;
innerClasses = null;
typeAnnotations = null;
methods = null;
fields = null;
recordComponents = null;
methodParams = null;
modulePackages = null;
moduleMainClass = null;
}
}

Expand Down
Loading
Loading