diff --git a/base/src/main/java/proguard/ClassSpecification.java b/base/src/main/java/proguard/ClassSpecification.java index ab6ae680..3d17e6e6 100644 --- a/base/src/main/java/proguard/ClassSpecification.java +++ b/base/src/main/java/proguard/ClassSpecification.java @@ -41,9 +41,9 @@ public class ClassSpecification implements Cloneable public final String extendsAnnotationType; public final String extendsClassName; - public final List attributeNames = null; - public List fieldSpecifications; - public List methodSpecifications; + public final List attributeNames = null; + public List fieldSpecifications; + public List methodSpecifications; /** diff --git a/base/src/main/java/proguard/ConfigurationParser.java b/base/src/main/java/proguard/ConfigurationParser.java index efa825d3..c0c2e265 100644 --- a/base/src/main/java/proguard/ConfigurationParser.java +++ b/base/src/main/java/proguard/ConfigurationParser.java @@ -1104,12 +1104,25 @@ private void parseMemberSpecificationArguments(String externalClassN // Parse the class member type and name part. // Did we get a special wildcard? - if (ConfigurationConstants.ANY_CLASS_MEMBER_KEYWORD.equals(nextWord) || - ConfigurationConstants.ANY_FIELD_KEYWORD .equals(nextWord) || - ConfigurationConstants.ANY_METHOD_KEYWORD .equals(nextWord)) + boolean isStar = ConfigurationConstants.ANY_CLASS_MEMBER_KEYWORD.equals(nextWord); + boolean isFields = ConfigurationConstants.ANY_FIELD_KEYWORD.equals(nextWord); + boolean isMethods = ConfigurationConstants.ANY_METHOD_KEYWORD.equals(nextWord); + boolean isFieldsOrMethods = isFields || isMethods; + + String type = nextWord; + String typeLocation = reader.locationDescription(); + + // Try to read the class member name; we need to do this now so that we can check the nextWord + // to see if we're parsing a wildcard type. + readNextWord("class member name", false, false, false); + + // Is it a wildcard star (short for all members) or is a type wildcard? + boolean isReallyStar = isStar && ConfigurationConstants.SEPARATOR_KEYWORD.equals(nextWord); + + if (isFieldsOrMethods || isReallyStar) { // Act according to the type of wildcard. - if (ConfigurationConstants.ANY_CLASS_MEMBER_KEYWORD.equals(nextWord)) + if (isStar) { checkFieldAccessFlags(requiredSetMemberAccessFlags, requiredUnsetMemberAccessFlags); @@ -1129,10 +1142,10 @@ private void parseMemberSpecificationArguments(String externalClassN null, null)); } - else if (ConfigurationConstants.ANY_FIELD_KEYWORD.equals(nextWord)) + else if (isFields) { checkFieldAccessFlags(requiredSetMemberAccessFlags, - requiredUnsetMemberAccessFlags); + requiredUnsetMemberAccessFlags); classSpecification.addField( new MemberSpecification(requiredSetMemberAccessFlags, @@ -1141,7 +1154,7 @@ else if (ConfigurationConstants.ANY_FIELD_KEYWORD.equals(nextWord)) null, null)); } - else if (ConfigurationConstants.ANY_METHOD_KEYWORD.equals(nextWord)) + else if (isMethods) { checkMethodAccessFlags(requiredSetMemberAccessFlags, requiredUnsetMemberAccessFlags); @@ -1154,9 +1167,6 @@ else if (ConfigurationConstants.ANY_METHOD_KEYWORD.equals(nextWord)) null)); } - // We still have to read the closing separator. - readNextWord("separator '" + ConfigurationConstants.SEPARATOR_KEYWORD + "'"); - if (!ConfigurationConstants.SEPARATOR_KEYWORD.equals(nextWord)) { throw new ParseException("Expecting separator '" + ConfigurationConstants.SEPARATOR_KEYWORD + @@ -1165,13 +1175,8 @@ else if (ConfigurationConstants.ANY_METHOD_KEYWORD.equals(nextWord)) } else { - // Make sure we have a proper type. - checkJavaIdentifier("java type"); - String type = nextWord; - String typeLocation = reader.locationDescription(); - - readNextWord("class member name"); String name = nextWord; + checkJavaIdentifier("java type", type, true); // Did we get just one word before the opening parenthesis? if (ConfigurationConstants.OPEN_ARGUMENTS_KEYWORD.equals(name)) @@ -1195,7 +1200,7 @@ else if (ConfigurationConstants.ANY_METHOD_KEYWORD.equals(nextWord)) { // It's not a constructor. // Make sure we have a proper name. - checkJavaIdentifier("class member name"); + checkNextWordIsJavaIdentifier("class member name"); // Read the opening parenthesis or the separating // semi-colon. @@ -1613,7 +1618,7 @@ else if (nextWord.equals(ConfigurationConstants.CLOSE_ARGUMENTS_KEYWORD)) { if (checkJavaIdentifiers) { - checkJavaIdentifier("java type", allowGenerics); + checkNextWordIsJavaIdentifier("java type", allowGenerics); } if (replaceSystemProperties) @@ -1880,30 +1885,34 @@ private boolean configurationEnd(boolean expectingAtCharacter) * Checks whether the given word is a valid Java identifier and throws * a ParseException if it isn't. Wildcard characters are accepted. */ - private void checkJavaIdentifier(String expectedDescription) + private void checkNextWordIsJavaIdentifier(String expectedDescription) throws ParseException { - checkJavaIdentifier(expectedDescription, true); + checkNextWordIsJavaIdentifier(expectedDescription, true); } + private void checkNextWordIsJavaIdentifier(String expectedDescription, boolean allowGenerics) throws ParseException + { + checkJavaIdentifier(expectedDescription, nextWord, allowGenerics); + } /** * Checks whether the given word is a valid Java identifier and throws * a ParseException if it isn't. Wildcard characters are accepted. */ - private void checkJavaIdentifier(String expectedDescription, boolean allowGenerics) - throws ParseException + private void checkJavaIdentifier(String expectedDescription, String identifier, boolean allowGenerics) + throws ParseException { - if (!isJavaIdentifier(nextWord)) + if (!isJavaIdentifier(identifier)) { throw new ParseException("Expecting " + expectedDescription + - " before " + reader.locationDescription()); + " before " + reader.locationDescription()); } - if (!allowGenerics && containsGenerics(nextWord)) + if (!allowGenerics && containsGenerics(identifier)) { throw new ParseException("Generics are not allowed (erased) in " + expectedDescription + - " " + reader.locationDescription()); + " " + reader.locationDescription()); } } diff --git a/base/src/test/kotlin/proguard/ClassSpecificationVisitorFactoryTest.kt b/base/src/test/kotlin/proguard/ClassSpecificationVisitorFactoryTest.kt index 800fa5a5..db8dbdf1 100644 --- a/base/src/test/kotlin/proguard/ClassSpecificationVisitorFactoryTest.kt +++ b/base/src/test/kotlin/proguard/ClassSpecificationVisitorFactoryTest.kt @@ -8,9 +8,14 @@ package proguard import io.kotest.core.spec.style.FreeSpec +import io.kotest.matchers.shouldBe import io.mockk.spyk import io.mockk.verify +import proguard.classfile.visitor.ClassPoolVisitor import proguard.classfile.visitor.ClassVisitor +import proguard.classfile.visitor.MemberCounter +import proguard.classfile.visitor.MemberVisitor +import proguard.testutils.AssemblerSource import proguard.testutils.ClassPoolBuilder import proguard.testutils.JavaSource import testutils.asConfiguration @@ -106,4 +111,110 @@ class ClassSpecificationVisitorFactoryTest : FreeSpec({ } } } + + "Given a class with fields of different types" - { + fun createFieldVisitor(config: String, fieldVisitor: MemberVisitor): ClassPoolVisitor = + KeepClassSpecificationVisitorFactory(true, false, false).createClassPoolVisitor( + config.asConfiguration().keep.first(), null, fieldVisitor, null, null + ) + + val (programClassPool, _) = ClassPoolBuilder.fromSource( + AssemblerSource( + "ClassWithFields.jbc", + """ + public class ClassWithFields { + java.lang.String myField; + int myField; + long myField; + com.example.ClassInAPackage myField; + java.lang.String[] myField; + int[] myField; + com.example.ClassInAPackage[] myField; + ClassWithFields myField; + } + """.trimIndent() + ) + ) + + "Then * should visit any non-primitive, non-array without package separator type fields" { + with(MemberCounter()) { + programClassPool.accept(createFieldVisitor("-keep class ClassWithFields { * myField; }", this)) + count shouldBe 1 + } + } + + "Then ** should visit any non-primitive, non-array separator type fields" { + with(MemberCounter()) { + programClassPool.accept(createFieldVisitor("-keep class ClassWithFields { ** myField; }", this)) + count shouldBe 3 + } + } + + "Then *** should visit all fields" { + with(MemberCounter()) { + programClassPool.accept(createFieldVisitor("-keep class ClassWithFields { *** myField; }", this)) + count shouldBe 8 + } + } + + "Then % should only visit primitive, non-array fields" { + with(MemberCounter()) { + programClassPool.accept(createFieldVisitor("-keep class ClassWithFields { % myField; }", this)) + count shouldBe 2 + } + } + } + + "Given a class with methods of different types" - { + fun createMethodVisitor(config: String, methodVisitor: MemberVisitor): ClassPoolVisitor = + KeepClassSpecificationVisitorFactory(true, false, false).createClassPoolVisitor( + config.asConfiguration().keep.first(), null, null, methodVisitor, null + ) + + val (programClassPool, _) = ClassPoolBuilder.fromSource( + AssemblerSource( + "ClassWithFields.jbc", + """ + public class ClassWithFields { + public void myMethod(java.lang.String); + public void myMethod(int); + public void myMethod(long); + public void myMethod(com.example.ClassInAPackage); + public void myMethod(java.lang.String[]); + public void myMethod(int[]); + public void myMethod(com.example.ClassInAPackage[]); + public void myMethod(ClassWithFields); + } + """.trimIndent() + ) + ) + + "Then * should visit any non-primitive, non-array without package separator type methods" { + with(MemberCounter()) { + programClassPool.accept(createMethodVisitor("-keep class ClassWithFields { void myMethod(*); }", this)) + count shouldBe 1 + } + } + + "Then ** should visit any non-primitive, non-array separator type methods" { + with(MemberCounter()) { + programClassPool.accept(createMethodVisitor("-keep class ClassWithFields { void myMethod(**); }", this)) + count shouldBe 3 + } + } + + "Then *** should visit all methods" { + with(MemberCounter()) { + programClassPool.accept(createMethodVisitor("-keep class ClassWithFields { void myMethod(***); }", this)) + count shouldBe 8 + } + } + + "Then % should only visit primitive, non-array methods" { + with(MemberCounter()) { + programClassPool.accept(createMethodVisitor("-keep class ClassWithFields { void myMethod(%); }", this)) + count shouldBe 2 + } + } + } }) diff --git a/base/src/test/kotlin/proguard/ConfigurationParserTest.kt b/base/src/test/kotlin/proguard/ConfigurationParserTest.kt index 82578e62..2d97a59e 100644 --- a/base/src/test/kotlin/proguard/ConfigurationParserTest.kt +++ b/base/src/test/kotlin/proguard/ConfigurationParserTest.kt @@ -9,6 +9,10 @@ package proguard import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.FreeSpec +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import proguard.classfile.AccessConstants.PUBLIC +import testutils.asConfiguration /** * Some simple testcases to catch special cases when parsing the Configuration. @@ -77,4 +81,184 @@ class ConfigurationParserTest : FreeSpec({ shouldThrow { parseConfiguration("-keep class * { (); }") } } } + + "Wildcard type tests" - { + class TestConfig( + val configOption: String, + classSpecificationConfig: String, + private val classSpecificationGetter: Configuration.() -> List? + ) { + private val configuration: Configuration by lazy { + "$configOption $classSpecificationConfig".asConfiguration() + } + val classSpecifications: List? get() = classSpecificationGetter.invoke(configuration) + } + + fun generateTestCases(clSpec: String): List = listOf( + TestConfig("-keep", clSpec) { keep }, + TestConfig("-assumenosideeffects", clSpec) { assumeNoSideEffects }, + TestConfig("-assumenoexternalsideeffects", clSpec) { assumeNoExternalSideEffects }, + TestConfig("-assumenoescapingparameters", clSpec) { assumeNoEscapingParameters }, + TestConfig("-assumenoexternalreturnvalues", clSpec) { assumeNoExternalReturnValues }, + TestConfig("-assumevalues", clSpec) { assumeValues }, + ) + + "Test wildcard matches all methods and fields" { + val testConfigurations = generateTestCases("class Foo { *; }") + generateTestCases("class Foo { ; ; }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications?.single() + methodSpecification shouldNotBe null + methodSpecification?.requiredSetAccessFlags shouldBe 0 + methodSpecification?.name shouldBe null + methodSpecification?.descriptor shouldBe null + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications?.single() + fieldSpecification shouldNotBe null + fieldSpecification?.requiredSetAccessFlags shouldBe 0 + fieldSpecification?.name shouldBe null + fieldSpecification?.descriptor shouldBe null + } + } + + "Test wildcard method return type" { + val testConfigurations = generateTestCases("class Foo { * bar(); }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications?.single() + methodSpecification?.requiredSetAccessFlags shouldBe 0 + methodSpecification?.name shouldBe "bar" + methodSpecification?.descriptor shouldBe "()L*;" + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications + fieldSpecification shouldBe null + } + } + + "Test wildcard method return type with access modifier" { + val testConfigurations = generateTestCases("class Foo { public * bar(); }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications?.single() + methodSpecification?.requiredSetAccessFlags shouldBe PUBLIC + methodSpecification?.name shouldBe "bar" + methodSpecification?.descriptor shouldBe "()L*;" + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications + fieldSpecification shouldBe null + } + } + + "Test wildcard field type" { + val testConfigurations = generateTestCases("class Foo { * bar; }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications + methodSpecification shouldBe null + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications?.single() + fieldSpecification?.requiredSetAccessFlags shouldBe 0 + fieldSpecification?.name shouldBe "bar" + fieldSpecification?.descriptor shouldBe "L*;" + } + } + + "Test wildcard field type with access modifier" { + val testConfigurations = generateTestCases("class Foo { public * bar; }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications + methodSpecification shouldBe null + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications?.single() + fieldSpecification?.requiredSetAccessFlags shouldBe PUBLIC + fieldSpecification?.name shouldBe "bar" + fieldSpecification?.descriptor shouldBe "L*;" + } + } + + "Test all type wildcard field" { + val testConfigurations = generateTestCases("class Foo { *** bar; }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications + methodSpecification shouldBe null + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications?.single() + fieldSpecification?.requiredSetAccessFlags shouldBe 0 + fieldSpecification?.name shouldBe "bar" + fieldSpecification?.descriptor shouldBe "L***;" + } + } + + "Test all type wildcard field type with access modifier" { + val testConfigurations = generateTestCases("class Foo { public *** bar; }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications + methodSpecification shouldBe null + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications?.single() + fieldSpecification?.requiredSetAccessFlags shouldBe PUBLIC + fieldSpecification?.name shouldBe "bar" + fieldSpecification?.descriptor shouldBe "L***;" + } + } + + "Test all type wildcard method return type" { + val testConfigurations = generateTestCases("class Foo { *** bar(); }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications?.single() + methodSpecification?.requiredSetAccessFlags shouldBe 0 + methodSpecification?.name shouldBe "bar" + methodSpecification?.descriptor shouldBe "()L***;" + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications + fieldSpecification shouldBe null + } + } + + "Test all type wildcard method return type with access modifier" { + val testConfigurations = generateTestCases("class Foo { public *** bar(); }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications?.single() + methodSpecification?.requiredSetAccessFlags shouldBe PUBLIC + methodSpecification?.name shouldBe "bar" + methodSpecification?.descriptor shouldBe "()L***;" + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications + fieldSpecification shouldBe null + } + } + + "Test concrete wildcard field type" { + val testConfigurations = generateTestCases("class Foo { java.lang.String bar; }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications + methodSpecification shouldBe null + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications?.single() + fieldSpecification?.requiredSetAccessFlags shouldBe 0 + fieldSpecification?.name shouldBe "bar" + fieldSpecification?.descriptor shouldBe "Ljava/lang/String;" + } + } + + "Test concrete wildcard method return type" { + val testConfigurations = generateTestCases("class Foo { java.lang.String bar(); }") + + for (testConfig in testConfigurations) { + val classSpecifications = testConfig.classSpecifications + val methodSpecification = classSpecifications?.single()?.methodSpecifications?.single() + methodSpecification?.requiredSetAccessFlags shouldBe 0 + methodSpecification?.name shouldBe "bar" + methodSpecification?.descriptor shouldBe "()Ljava/lang/String;" + val fieldSpecification = classSpecifications?.single()?.fieldSpecifications + fieldSpecification shouldBe null + } + } + } }) diff --git a/base/src/test/kotlin/proguard/optimize/peephole/MethodInlinerJava9Test.kt b/base/src/test/kotlin/proguard/optimize/peephole/MethodInlinerJava9Test.kt index 38a44664..40aa0d59 100644 --- a/base/src/test/kotlin/proguard/optimize/peephole/MethodInlinerJava9Test.kt +++ b/base/src/test/kotlin/proguard/optimize/peephole/MethodInlinerJava9Test.kt @@ -48,10 +48,10 @@ class MethodInlinerJava9Test : FreeSpec({ // Initialize optimization info (used when inlining). val optimizationInfoInitializer: ClassVisitor = MultiClassVisitor( - ProgramClassOptimizationInfoSetter(), - AllMethodVisitor( - ProgramMemberOptimizationInfoSetter() - ) + ProgramClassOptimizationInfoSetter(), + AllMethodVisitor( + ProgramMemberOptimizationInfoSetter() + ) ) programClassPool.classesAccept(optimizationInfoInitializer) @@ -63,11 +63,11 @@ class MethodInlinerJava9Test : FreeSpec({ "Then the interface method is inlined" { programClassPool.classesAccept( - AllMethodVisitor( - AllAttributeVisitor( - methodInliner - ) + AllMethodVisitor( + AllAttributeVisitor( + methodInliner ) + ) ) val lengthAfter = codeAttr.u4codeLength diff --git a/base/src/test/kotlin/proguard/optimize/peephole/MethodInlinerTest.kt b/base/src/test/kotlin/proguard/optimize/peephole/MethodInlinerTest.kt index 975e0844..f380fc92 100644 --- a/base/src/test/kotlin/proguard/optimize/peephole/MethodInlinerTest.kt +++ b/base/src/test/kotlin/proguard/optimize/peephole/MethodInlinerTest.kt @@ -285,9 +285,9 @@ class MethodInlinerTest : FreeSpec({ "Given a method calling another non-private method in an interface" - { val (programClassPool, _) = ClassPoolBuilder.fromSource( - JavaSource( - "Foo.java", - """interface Foo { + JavaSource( + "Foo.java", + """interface Foo { default void f1() { f2(); } @@ -309,10 +309,10 @@ class MethodInlinerTest : FreeSpec({ // Initialize optimization info (used when inlining). val optimizationInfoInitializer: ClassVisitor = MultiClassVisitor( - ProgramClassOptimizationInfoSetter(), - AllMethodVisitor( - ProgramMemberOptimizationInfoSetter() - ) + ProgramClassOptimizationInfoSetter(), + AllMethodVisitor( + ProgramMemberOptimizationInfoSetter() + ) ) programClassPool.classesAccept(optimizationInfoInitializer) @@ -324,11 +324,11 @@ class MethodInlinerTest : FreeSpec({ "Then the interface method is not inlined" { programClassPool.classesAccept( - AllMethodVisitor( - AllAttributeVisitor( - methodInliner - ) + AllMethodVisitor( + AllAttributeVisitor( + methodInliner ) + ) ) val lengthAfter = codeAttr.u4codeLength diff --git a/docs/md/manual/releasenotes.md b/docs/md/manual/releasenotes.md index 16e8d48b..898c91ec 100644 --- a/docs/md/manual/releasenotes.md +++ b/docs/md/manual/releasenotes.md @@ -15,6 +15,10 @@ - Fix potential duplication class when name obfuscating Kotlin multi-file facades. - Do not inline interface methods during optimization to avoid compilation errors during output writing due to an interface method being made package visible. +### Added + +- Support parsing of wildcard `*` when used as a field type or method return type in class specifications. + ## Version 7.3.2 ### Java support