diff --git a/src/org/jetbrains/java/decompiler/main/collectors/ImportCollector.java b/src/org/jetbrains/java/decompiler/main/collectors/ImportCollector.java index 3bc5383..f5cd16d 100644 --- a/src/org/jetbrains/java/decompiler/main/collectors/ImportCollector.java +++ b/src/org/jetbrains/java/decompiler/main/collectors/ImportCollector.java @@ -15,10 +15,13 @@ */ package org.jetbrains.java.decompiler.main.collectors; +import org.jetbrains.java.decompiler.main.ClassesProcessor; import org.jetbrains.java.decompiler.main.ClassesProcessor.ClassNode; import org.jetbrains.java.decompiler.main.DecompilerContext; import org.jetbrains.java.decompiler.main.TextBuffer; +import org.jetbrains.java.decompiler.struct.StructClass; import org.jetbrains.java.decompiler.struct.StructContext; +import org.jetbrains.java.decompiler.struct.StructField; import java.util.*; import java.util.stream.Collectors; @@ -28,6 +31,8 @@ public class ImportCollector { private final Map mapSimpleNames = new HashMap<>(); private final Set setNotImportedNames = new HashSet<>(); + // set of field names in this class and all its predecessors. + private final Set setFieldNames = new HashSet<>(); private final String currentPackageSlash; private final String currentPackagePoint; @@ -43,6 +48,37 @@ public class ImportCollector { currentPackageSlash = ""; currentPackagePoint = ""; } + + Map mapRootCases = DecompilerContext.getClassProcessor().getMapRootClasses(); + for(StructClass sClass = root.classStruct; + sClass!=null; + ){ + // all field names for current class .. + for(StructField f: sClass.getFields()) { + setFieldNames.add(f.getName()); + } + + // .. and traverse through parent. + ClassNode classNode; + if(sClass.superClass==null || (classNode = (mapRootCases.get(sClass.superClass.getString())))==null) + break; + sClass = classNode.classStruct; + } + } + + /** + * Check whether the package-less name ClassName is shaded by variable in a context of + * the decompiled class + * @param classToName - pkg.name.ClassName - class to find shortname for + * @return ClassName if the name is not shaded by local field, pkg.name.ClassName otherwise + */ + public String getShortNameInClassContext(String classToName) { + String shortName = getShortName(classToName); + if(setFieldNames.contains(shortName)) { + return classToName; + } else { + return shortName; + } } public String getShortName(String fullName) { diff --git a/src/org/jetbrains/java/decompiler/modules/decompiler/exps/FieldExprent.java b/src/org/jetbrains/java/decompiler/modules/decompiler/exps/FieldExprent.java index 472823f..6d549b1 100644 --- a/src/org/jetbrains/java/decompiler/modules/decompiler/exps/FieldExprent.java +++ b/src/org/jetbrains/java/decompiler/modules/decompiler/exps/FieldExprent.java @@ -102,7 +102,7 @@ public class FieldExprent extends Exprent { if (isStatic) { ClassNode node = (ClassNode)DecompilerContext.getProperty(DecompilerContext.CURRENT_CLASS_NODE); if (node == null || !classname.equals(node.classStruct.qualifiedName) || isAmbiguous()) { - buf.append(DecompilerContext.getImportCollector().getShortName(ExprProcessor.buildJavaClassName(classname))); + buf.append(DecompilerContext.getImportCollector().getShortNameInClassContext(ExprProcessor.buildJavaClassName(classname))); buf.append("."); } } diff --git a/src/org/jetbrains/java/decompiler/modules/decompiler/exps/InvocationExprent.java b/src/org/jetbrains/java/decompiler/modules/decompiler/exps/InvocationExprent.java index dd8e26e..f56a805 100644 --- a/src/org/jetbrains/java/decompiler/modules/decompiler/exps/InvocationExprent.java +++ b/src/org/jetbrains/java/decompiler/modules/decompiler/exps/InvocationExprent.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2016 JetBrains s.r.o. + * Copyright 2000-2017 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -225,7 +225,7 @@ public class InvocationExprent extends Exprent { ClassNode node = (ClassNode)DecompilerContext.getProperty(DecompilerContext.CURRENT_CLASS_NODE); if (node == null || !classname.equals(node.classStruct.qualifiedName)) { - buf.append(DecompilerContext.getImportCollector().getShortName(ExprProcessor.buildJavaClassName(classname))); + buf.append(DecompilerContext.getImportCollector().getShortNameInClassContext(ExprProcessor.buildJavaClassName(classname))); } } else { diff --git a/test/org/jetbrains/java/decompiler/SingleClassesTest.java b/test/org/jetbrains/java/decompiler/SingleClassesTest.java index ba2429f..b4567e0 100644 --- a/test/org/jetbrains/java/decompiler/SingleClassesTest.java +++ b/test/org/jetbrains/java/decompiler/SingleClassesTest.java @@ -102,6 +102,11 @@ public class SingleClassesTest { @Test public void testAccessReplace() { doTest("pkg/TestAccessReplace"); } @Test public void testStringLiterals() { doTest("pkg/TestStringLiterals"); } @Test public void testPrimitives() { doTest("pkg/TestPrimitives"); } + @Test public void testClashName() { doTest("pkg/TestClashName", "pkg/SharedName1", + "pkg/SharedName2", "pkg/SharedName3", "pkg/SharedName4", "pkg/NonSharedName", + "pkg/TestClashNameParent", "ext/TestClashNameParent","pkg/TestClashNameIface", "ext/TestClashNameIface"); } + @Test public void testSwitchOnEnum() { doTest("pkg/TestSwitchOnEnum","pkg/TestSwitchOnEnum$1");} + private void doTest(String testFile, String... companionFiles) { ConsoleDecompiler decompiler = fixture.getDecompiler(); diff --git a/testData/classes/ext/TestClashNameIface.class b/testData/classes/ext/TestClashNameIface.class new file mode 100644 index 0000000..6a725cf Binary files /dev/null and b/testData/classes/ext/TestClashNameIface.class differ diff --git a/testData/classes/ext/TestClashNameParent.class b/testData/classes/ext/TestClashNameParent.class new file mode 100644 index 0000000..4187761 Binary files /dev/null and b/testData/classes/ext/TestClashNameParent.class differ diff --git a/testData/classes/pkg/NonSharedName.class b/testData/classes/pkg/NonSharedName.class new file mode 100644 index 0000000..e3d09dd Binary files /dev/null and b/testData/classes/pkg/NonSharedName.class differ diff --git a/testData/classes/pkg/SharedName1.class b/testData/classes/pkg/SharedName1.class new file mode 100644 index 0000000..fbd917f Binary files /dev/null and b/testData/classes/pkg/SharedName1.class differ diff --git a/testData/classes/pkg/SharedName2.class b/testData/classes/pkg/SharedName2.class new file mode 100644 index 0000000..275ab11 Binary files /dev/null and b/testData/classes/pkg/SharedName2.class differ diff --git a/testData/classes/pkg/SharedName3.class b/testData/classes/pkg/SharedName3.class new file mode 100644 index 0000000..f24863d Binary files /dev/null and b/testData/classes/pkg/SharedName3.class differ diff --git a/testData/classes/pkg/SharedName4.class b/testData/classes/pkg/SharedName4.class new file mode 100644 index 0000000..2bfb786 Binary files /dev/null and b/testData/classes/pkg/SharedName4.class differ diff --git a/testData/classes/pkg/SharedName5.class b/testData/classes/pkg/SharedName5.class new file mode 100644 index 0000000..f975840 Binary files /dev/null and b/testData/classes/pkg/SharedName5.class differ diff --git a/testData/classes/pkg/TestClashName.class b/testData/classes/pkg/TestClashName.class new file mode 100644 index 0000000..73b047d Binary files /dev/null and b/testData/classes/pkg/TestClashName.class differ diff --git a/testData/classes/pkg/TestClashNameIface.class b/testData/classes/pkg/TestClashNameIface.class new file mode 100644 index 0000000..eeb153c Binary files /dev/null and b/testData/classes/pkg/TestClashNameIface.class differ diff --git a/testData/classes/pkg/TestClashNameParent.class b/testData/classes/pkg/TestClashNameParent.class new file mode 100644 index 0000000..4c4119d Binary files /dev/null and b/testData/classes/pkg/TestClashNameParent.class differ diff --git a/testData/classes/pkg/TestSwitchOnEnum$1.class b/testData/classes/pkg/TestSwitchOnEnum$1.class new file mode 100644 index 0000000..a3f6924 Binary files /dev/null and b/testData/classes/pkg/TestSwitchOnEnum$1.class differ diff --git a/testData/classes/pkg/TestSwitchOnEnum.class b/testData/classes/pkg/TestSwitchOnEnum.class new file mode 100644 index 0000000..c377fe7 Binary files /dev/null and b/testData/classes/pkg/TestSwitchOnEnum.class differ diff --git a/testData/results/InvalidMethodSignature.dec b/testData/results/InvalidMethodSignature.dec index e2a15b3..e4b567c 100644 --- a/testData/results/InvalidMethodSignature.dec +++ b/testData/results/InvalidMethodSignature.dec @@ -17,7 +17,7 @@ class i implements bg { public void a(c var1, k var2, boolean var3) { File var4 = this.a.b().a(var1);// 2 - b.a(this.b).add(var4);// 3 + a.a.a.a.e.f.b.a(this.b).add(var4);// 3 }// 4 public void a(a.a.a.a.c.b var1) { diff --git a/testData/results/TestClashName.dec b/testData/results/TestClashName.dec new file mode 100644 index 0000000..2a9a731 --- /dev/null +++ b/testData/results/TestClashName.dec @@ -0,0 +1,108 @@ +package pkg; + +@SharedName4 +public class TestClashName extends ext.TestClashNameParent implements TestClashNameIface { + int TestClashNameParent = 0; + int TestClashNameIface = 0; + int SharedName1 = 0; + int SharedName4 = 0; + int SharedName5 = 0; + int i; + int j; + int k; + int l; + int m; + int n; + SharedName1 p; + SharedName5 q; + + public TestClashName() { + this.i = pkg.SharedName1.f;// 59 + this.j = NonSharedName.f;// 60 + this.k = SharedName2.f;// 61 + this.l = pkg.SharedName3.f;// 62 + this.m = pkg.SharedName1.getF();// 63 + this.n = NonSharedName.getF();// 64 + this.p = null;// 65 + this.q = null;// 67 + } + + @SharedName4 + public int m() { + int var1 = this.i;// 73 + pkg.SharedName1.f = this.j;// 74 + int var2 = SharedName2.f;// 75 + NonSharedName.f = this.k;// 76 + int var3 = NonSharedName.f;// 77 + return var1 + var2 + var3;// 78 + } + + public void f() { + }// 82 +} + +class 'pkg/TestClashName' { + method ' ()V' { + 1e 19 + 21 19 + 25 20 + 28 20 + 2c 21 + 2f 21 + 33 22 + 36 22 + 3a 23 + 3d 23 + 41 24 + 44 24 + 48 25 + 49 25 + 4d 26 + 4e 26 + 51 27 + } + + method 'm ()I' { + 1 31 + 4 31 + 6 32 + 9 32 + c 33 + f 33 + 11 34 + 14 34 + 17 35 + 1a 35 + 1d 36 + 1f 36 + 20 36 + } + + method 'f ()V' { + 0 40 + } +} + +Lines mapping: +59 <-> 20 +60 <-> 21 +61 <-> 22 +62 <-> 23 +63 <-> 24 +64 <-> 25 +65 <-> 26 +67 <-> 27 +73 <-> 32 +74 <-> 33 +75 <-> 34 +76 <-> 35 +77 <-> 36 +78 <-> 37 +82 <-> 41 +Not mapped: +52 +53 +54 +55 +56 +57 diff --git a/testData/results/TestSwitchOnEnum.dec b/testData/results/TestSwitchOnEnum.dec new file mode 100644 index 0000000..4a6d980 --- /dev/null +++ b/testData/results/TestSwitchOnEnum.dec @@ -0,0 +1,31 @@ +package pkg; + +import java.util.concurrent.TimeUnit; + +public class TestSwitchOnEnum { + int myInt; + + public void testSOE(TimeUnit var1) { + switch(null.$SwitchMap$java$util$concurrent$TimeUnit[var1.ordinal()]) {// 12 + case 1: + return;// 14 + default: + } + }// 16 +} + +class 'pkg/TestSwitchOnEnum' { + method 'testSOE (Ljava/util/concurrent/TimeUnit;)V' { + 0 8 + 4 8 + 7 8 + 8 8 + 1c 10 + 1d 13 + } +} + +Lines mapping: +12 <-> 9 +14 <-> 11 +16 <-> 14 diff --git a/testData/src/ext/TestClashNameIface.java b/testData/src/ext/TestClashNameIface.java new file mode 100644 index 0000000..9d7630b --- /dev/null +++ b/testData/src/ext/TestClashNameIface.java @@ -0,0 +1,4 @@ +package ext; +interface TestClashNameIface { + public void foo(); +} diff --git a/testData/src/ext/TestClashNameParent.java b/testData/src/ext/TestClashNameParent.java new file mode 100644 index 0000000..7691344 --- /dev/null +++ b/testData/src/ext/TestClashNameParent.java @@ -0,0 +1,5 @@ +package ext; + +public class TestClashNameParent { + int SharedName3 = 0; +} diff --git a/testData/src/pkg/TestClashName.java b/testData/src/pkg/TestClashName.java new file mode 100644 index 0000000..3b13448 --- /dev/null +++ b/testData/src/pkg/TestClashName.java @@ -0,0 +1,83 @@ +package pkg; + +/* + * The names SharedName[123] are shared between variables in local|parent class and class names. + * Where approprate, the classes have to be referenced by fully qualified names + */ + +class SharedName1 { + static int f = 0; + + static int getF() { + return f; + } +} + +class SharedName2 { + static int f = 0; +} + +class SharedName3 { + static int f = 0; +} + +class NonSharedName { + static int f = 0; + + static int getF() { + return f; + } +} + +@interface SharedName4 { +} + +class SharedName5{ +} + +class TestClashNameParent { + +} + +interface TestClashNameIface { + public void f(); +} +// *** Legend for first sentence in comments below: +// (+) or (-) indicate whether 'pkg' prefix is or is not required when referencing *SharedName*. +// The sign is optionally followed by decompiler class name that generates the line that is being commented +// in a call of getShortName() + +@SharedName4 // (-)AnnotationExprent. While a variable named SharedName4 does exist in the annotated class, + // lookup process for annotation names never includes variable names. +public class TestClashName extends ext.TestClashNameParent implements /*pkg.*/TestClashNameIface { // (+)(-)TextUtil + int TestClashNameParent = 0; + int TestClashNameIface = 0; + int SharedName1 = 0; + int SharedName4 = 0; + int SharedName5 = 0; + + int i = pkg.SharedName1.f; // (+)FieldExprent. SharedName1 class name is shadowed by a variable in this class + int j = NonSharedName.f; // (-)FieldExprent. The NonSharedName is not used for other objects in the current scope + int k = SharedName2.f; // (-)FieldExprent. SharedName2 variable is not the current scope + int l = pkg.SharedName3.f; // (+)FieldExprent. SharedName3 class name is shadowed by a variable in parent class + int m = pkg.SharedName1.getF();// (+)InvocationExprent. SharedName1 class name is shadowed by a variable in this class + int n = NonSharedName.getF(); // (-)InvocationExprent. The NonSharedName is not used for other objects in the current scope + SharedName1 p = null; // (-)ExprProcessor. While a variable named SharedName1 in current scope does exist, + // namespace in type declaration does not include variable names in a scope + SharedName5 q = null;//(-)(-)GenericMain (both names). While a variable named SharedName1 does exist in current scope, + // lookup for generic parameters never includes variable names + + @SharedName4 // (-)AnnotationExprent. While a variable named SharedName4 does exist in current scope, + // lookup process for annotation names never includes variable names. + public int m() { + int SharedName2 = i; + pkg.SharedName1.f = j; // (+)FieldExprent. SharedName1 class name is shadowed by a variable in this class + int x = pkg.SharedName2.f; // (+)FieldExprent. SharedName2 class name is shadowed by a variable in this method + NonSharedName.f = k; // (-)ExprProcessor. The NonSharedName is not used for other objects in the current scope + int y = NonSharedName.f; // (-)FieldExprent. The NonSharedName is not used for other objects in the current scope + return SharedName2 + x + y; + } + + @Override + public void f() {} +} diff --git a/testData/src/pkg/TestSwitchOnEnum.java b/testData/src/pkg/TestSwitchOnEnum.java new file mode 100644 index 0000000..ec07604 --- /dev/null +++ b/testData/src/pkg/TestSwitchOnEnum.java @@ -0,0 +1,20 @@ +package pkg; + +import java.util.concurrent.TimeUnit; + +/** + * This illustrates a bug in fernflower as of 20170421. Decompiled output of this class does not compile back. + */ +public class TestSwitchOnEnum { + + int myInt; + + public int testSOE(TimeUnit t) { + // This creates anonymous SwitchMap inner class. + switch (t) { + case SECONDS: + return 1; + } + return 0; + } +}