From ec452e58a5642de2d7a41bfcece43df1fec51d16 Mon Sep 17 00:00:00 2001 From: Antony Sargent Date: Thu, 31 Aug 2017 13:34:06 -0700 Subject: [PATCH] Fix ClassScanner and re-enable CodeInspectionTest tests This fixes the code in ClassScanner for finding all classes in a given package to not depend on directory entries in the .jar files generated by the build system. This dependency caused our tests in CodeInspepectionTest.java to fail when this CL: https://android-review.googlesource.com/#/c/platform/build/+/456418/ stopped adding directory entries in the .jar files generated by the build process. Instead of depending on directories being present in the list of resources provided by the classloader, this CL switches to using Guava's ClassPath class to enumerate all loadable classes and filter them to the ones in the package of interest. Change-Id: I583919096450b61d4816256be280e2f5f1ce2316 Fixes: 64840107 Test: make RunSettingsRoboTests --- .../core/codeinspection/ClassScanner.java | 114 ++++-------------- .../codeinspection/CodeInspectionTest.java | 8 +- 2 files changed, 29 insertions(+), 93 deletions(-) diff --git a/tests/robotests/src/com/android/settings/core/codeinspection/ClassScanner.java b/tests/robotests/src/com/android/settings/core/codeinspection/ClassScanner.java index 09af870fdfd..bf57f406e3a 100644 --- a/tests/robotests/src/com/android/settings/core/codeinspection/ClassScanner.java +++ b/tests/robotests/src/com/android/settings/core/codeinspection/ClassScanner.java @@ -16,50 +16,47 @@ package com.android.settings.core.codeinspection; -import java.io.File; +import com.google.common.reflect.ClassPath; + import java.io.IOException; -import java.net.JarURLConnection; -import java.net.URL; -import java.net.URLConnection; -import java.net.URLDecoder; import java.util.ArrayList; -import java.util.Enumeration; import java.util.List; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * Scans and builds all classes in current classloader. */ public class ClassScanner { - private static final String CLASS_SUFFIX = ".class"; - public List> getClassesForPackage(String packageName) throws ClassNotFoundException { final List> classes = new ArrayList<>(); try { - final Enumeration resources = Thread.currentThread().getContextClassLoader() - .getResources(packageName.replace('.', '/')); - if (!resources.hasMoreElements()) { - return classes; - } - URL url = resources.nextElement(); - while (url != null) { - final URLConnection connection = url.openConnection(); + ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); + ClassPath classPath = ClassPath.from(classLoader); - if (connection instanceof JarURLConnection) { - loadClassFromJar((JarURLConnection) connection, packageName, - classes); - } else { - loadClassFromDirectory(new File(URLDecoder.decode(url.getPath(), "UTF-8")), - packageName, classes); - } - if (resources.hasMoreElements()) { - url = resources.nextElement(); - } else { - break; + // Some anonymous classes don't return true when calling isAnonymousClass(), but they + // always seem to be nested anonymous classes like com.android.settings.Foo$1$2. In + // general we don't want any anonymous classes so we just filter these out by searching + // for $[0-9] in the name. + Pattern anonymousClassPattern = Pattern.compile(".*\\$\\d+.*"); + Matcher anonymousClassMatcher = anonymousClassPattern.matcher(""); + + for (ClassPath.ClassInfo info : classPath.getAllClasses()) { + if (info.getPackageName().startsWith(packageName)) { + try { + Class clazz = classLoader.loadClass(info.getName()); + if (clazz.isAnonymousClass() || anonymousClassMatcher.reset( + clazz.getName()).matches()) { + continue; + } + classes.add(clazz); + } catch (NoClassDefFoundError e) { + // do nothing. this class hasn't been found by the + // loader, and we don't care. + } } } } catch (final IOException e) { @@ -68,63 +65,4 @@ public class ClassScanner { return classes; } - private void loadClassFromDirectory(File directory, String packageName, List> classes) - throws ClassNotFoundException { - if (directory.exists() && directory.isDirectory()) { - final String[] files = directory.list(); - - for (final String file : files) { - if (file.endsWith(CLASS_SUFFIX)) { - try { - classes.add(Class.forName( - packageName + '.' + file.substring(0, file.length() - 6), - false /* init */, - Thread.currentThread().getContextClassLoader())); - } catch (NoClassDefFoundError e) { - // do nothing. this class hasn't been found by the - // loader, and we don't care. - } - } else { - final File tmpDirectory = new File(directory, file); - if (tmpDirectory.isDirectory()) { - loadClassFromDirectory(tmpDirectory, packageName + "." + file, classes); - } - } - } - } - } - - private void loadClassFromJar(JarURLConnection connection, String packageName, - List> classes) throws ClassNotFoundException, IOException { - final JarFile jarFile = connection.getJarFile(); - final Enumeration entries = jarFile.entries(); - String name; - if (!entries.hasMoreElements()) { - return; - } - JarEntry jarEntry = entries.nextElement(); - while (jarEntry != null) { - name = jarEntry.getName(); - - if (name.contains(CLASS_SUFFIX)) { - name = name.substring(0, name.length() - CLASS_SUFFIX.length()).replace('/', '.'); - - if (name.startsWith(packageName)) { - try { - classes.add(Class.forName(name, - false /* init */, - Thread.currentThread().getContextClassLoader())); - } catch (NoClassDefFoundError e) { - // do nothing. this class hasn't been found by the - // loader, and we don't care. - } - } - } - if (entries.hasMoreElements()) { - jarEntry = entries.nextElement(); - } else { - break; - } - } - } } diff --git a/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspectionTest.java b/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspectionTest.java index faaf338d15b..126a346da02 100644 --- a/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspectionTest.java +++ b/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspectionTest.java @@ -44,14 +44,12 @@ public class CodeInspectionTest { @Before public void setUp() throws Exception { mClasses = new ClassScanner().getClassesForPackage(CodeInspector.PACKAGE_NAME); - // Disabled temporarily - see b/64840107 - //assertThat(mClasses).isNotEmpty(); + assertThat(mClasses).isNotEmpty(); } @Test public void runCodeInspections() { - // Disabled temporarily - see b/64840107 - // new InstrumentableFragmentCodeInspector(mClasses).run(); - // new SearchIndexProviderCodeInspector(mClasses).run(); + new InstrumentableFragmentCodeInspector(mClasses).run(); + new SearchIndexProviderCodeInspector(mClasses).run(); } }