Use fully qualified static field name in conflict #541

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<String, String> mapSimpleNames = new HashMap<>();
   private final Set<String> setNotImportedNames = new HashSet<>();
+  // set of field names in this class and all its predecessors.
+  private final Set<String> setFieldNames = new HashSet<>();
   private final String currentPackageSlash;
   private final String currentPackagePoint;
 
@@ -43,6 +48,37 @@ public class ImportCollector {
       currentPackageSlash = "";
       currentPackagePoint = "";
     }
+
+    Map<String, ClassNode> 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) {

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(".");
       }
     }

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 {

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();

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) {

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<SharedName1> 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 '<init> ()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

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

testData/src/ext/TestClashNameIface.java

@@ -0,0 +1,4 @@
+package ext;
+interface TestClashNameIface {
+  public void foo();
+}

testData/src/ext/TestClashNameParent.java

@@ -0,0 +1,5 @@
+package ext;
+
+public class TestClashNameParent {
+  int SharedName3 = 0;
+}

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<T>{
+}
+
+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<SharedName1> 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() {}
+}

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;
+  }
+}