Revert "[Xamarin.Android.Tools.Bytecode] hide nested types (#827)" (#855

)

Revert commit 4ef5081.

Fixes: #854

Context: 4ef5081
Context: #826

[As noted][0] in [PR #827][1], there is some "weirdness" with what
appears in the [`InnerClasses` collection][2].  It turns out this is
due to a misunderstanding of what the `InnerClasses` collection
contains.

As per the [docs][2]]:

> If a class has members that are classes or interfaces, its
> `constant_pool` table (and hence its `InnerClasses` attribute) must
> refer to each such member, even if that member is not otherwise
> mentioned by the class.
> **These rules imply that a nested class or interface member will
> have `InnerClasses` information for each enclosing class and for
> each immediate member.**

(Emphasis added.)  That is, the `PagedList$Config$Builder$Companion`
class lists *both* its immediate containing type
`PagedList$Config$Builder` and the "parent-parent" containing type
`PagedList$Config` within `InnerClasses`.

The change made in commit 4ef5081 loops through `InnerClasses`,
marking them as `internal`, assuming they are all nested types.
This is causing us to hide *declaring* types when we are trying to
hide *nested* types.

This will require more investigation and the deadline for 16.11 is
~now, so we're just going to revert the original commit.

[0]: #827 (comment)
[1]: #827
[2]: https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.6

src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs

@@ -30,7 +30,7 @@ public static void Fixup (IList<ClassFile> classes)
 					var class_metadata = (metadata as KotlinClass);
 
 					if (class_metadata != null) {
-						FixupClassVisibility (c, class_metadata, classes);
+						FixupClassVisibility (c, class_metadata);
 
 						if (!c.AccessFlags.IsPubliclyVisible ())
 							continue;
@@ -62,7 +62,7 @@ public static void Fixup (IList<ClassFile> classes)
 			}
 		}
 
-		static void FixupClassVisibility (ClassFile klass, KotlinClass metadata, IList<ClassFile> classes)
+		static void FixupClassVisibility (ClassFile klass, KotlinClass metadata)
 		{
 			// Hide class if it isn't Public/Protected
 			if (klass.AccessFlags.IsPubliclyVisible () && !metadata.Visibility.IsPubliclyVisible ()) {
@@ -83,33 +83,15 @@ static void FixupClassVisibility (ClassFile klass, KotlinClass metadata, IList<C
 				Log.Debug ($"Kotlin: Hiding internal class {klass.ThisClass.Name.Value}");
 				klass.AccessFlags = SetVisibility (klass.AccessFlags, ClassAccessFlags.Private);
 
-				foreach (var ic in klass.InnerClasses)
-					HideInternalInnerClass (ic, classes);
+				foreach (var ic in klass.InnerClasses) {
+					Log.Debug ($"Kotlin: Hiding nested internal type {ic.InnerClass.Name.Value}");
+					ic.InnerClassAccessFlags = SetVisibility (ic.InnerClassAccessFlags, ClassAccessFlags.Private);
+				}
 
 				return;
 			}
 		}
 
-		static void HideInternalInnerClass (InnerClassInfo klass, IList<ClassFile> classes)
-		{
-			var existing = klass.InnerClassAccessFlags;
-
-			klass.InnerClassAccessFlags = SetVisibility (existing, ClassAccessFlags.Private);
-			Log.Debug ($"Kotlin: Hiding nested internal type {klass.InnerClass.Name.Value}");
-
-			// Setting the inner class access flags above doesn't technically do anything, because we output
-			// from the inner class's ClassFile, so we need to find that and set it there.
-			var kf = classes.FirstOrDefault (c => c.ThisClass.Name.Value == klass.InnerClass.Name.Value);
-
-			if (kf != null) {
-				kf.AccessFlags = SetVisibility (kf.AccessFlags, ClassAccessFlags.Private);
-				kf.InnerClass.InnerClassAccessFlags = SetVisibility (kf.InnerClass.InnerClassAccessFlags, ClassAccessFlags.Private);
-
-				foreach (var inner_class in kf.InnerClasses.Where (c => c.OuterClass.Name.Value == kf.ThisClass.Name.Value))
-					HideInternalInnerClass (inner_class, classes);
-			}
-		}
-
 		// Passing null for 'newVisibility' parameter means 'package-private'
 		static ClassAccessFlags SetVisibility (ClassAccessFlags existing, ClassAccessFlags? newVisibility)
 		{

tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs

@@ -310,24 +310,5 @@ public void HandleKotlinNameShadowing ()
 			Assert.True (klass.Methods.Single (m => m.Name == "hitCount").AccessFlags.HasFlag (MethodAccessFlags.Public));
 			Assert.True (klass.Methods.Single (m => m.Name == "setType").AccessFlags.HasFlag (MethodAccessFlags.Public));
 		}
-
-		[Test]
-		public void HidePublicInterfaceInInternalClass ()
-		{
-			var klass = LoadClassFile ("InternalClassWithNestedInterface.class");
-			var inner_interface = LoadClassFile ("InternalClassWithNestedInterface$NestedInterface.class");
-			var inner_inner_interface = LoadClassFile ("InternalClassWithNestedInterface$NestedInterface$DoubleNestedInterface.class");
-
-			KotlinFixups.Fixup (new [] { klass, inner_interface, inner_inner_interface });
-
-			var output = new XmlClassDeclarationBuilder (klass).ToXElement ().ToString ();
-			Assert.True (output.Contains ("visibility=\"public\""));
-
-			var output2 = new XmlClassDeclarationBuilder (inner_interface).ToXElement ().ToString ();
-			Assert.True (output2.Contains ("visibility=\"private\""));
-
-			var output3 = new XmlClassDeclarationBuilder (inner_inner_interface).ToXElement ().ToString ();
-			Assert.True (output3.Contains ("visibility=\"private\""));
-		}
 	}
 }