924060929 commented on code in PR #67824: URL: https://github.com/apache/doris/pull/67824#discussion_r3987834918
########## fe/fe-core/src/main/java/org/apache/doris/buildtools/IncrementalSourceMarker.java: ########## @@ -0,0 +1,588 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.buildtools; + +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.attribute.FileTime; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; + +/** + * Computes the set of sources which must be recompiled after a local edit and marks them for the + * compiler plugin, so that {@code mvn compile} stops rebuilding the whole fe-core module. + * + * <p>Background: {@code maven-compiler-plugin} is all-or-nothing by default (one stale source + * recompiles every source of the module), and its {@code useIncrementalCompilation=false} mode, + * which does compile only the stale sources, is unsafe on its own because it never recompiles the + * classes which <em>use</em> a changed class. + * + * <p>This tool closes that gap. For every scanned source file it caches, keyed by the content hash: + * <ul> + * <li>the identifiers the file mentions, which over-approximates the types it depends on,</li> + * <li>the type names the file declares.</li> + * </ul> + * A run then computes + * <pre> + * changed = files whose content changed, plus files whose set of declared names changed + * (a type was added, removed, renamed or moved) + * affected = changed, closed over "mentions a type name declared by an affected file" + * </pre> + * and updates the last modification time of every affected file which belongs to the main sources. + * The compiler plugin then compiles exactly that set. Because every file which mentions a changed + * type is touched, no class is left referring to a type which was changed or deleted underneath it; + * the set is a conservative superset, so the only cost of an imprecise name match is a few extra + * files being recompiled. + * + * <p>The compiler plugin still owns the compilation, its annotation processing and its classpath. + * This tool only decides which sources are stale. + */ +public class IncrementalSourceMarker { + /** Bump it whenever the cached facts change, then the index is rebuilt from scratch. */ + private static final int FORMAT_VERSION = 1; + + /** The java keywords which introduce a type name. {@code interface} also covers {@code @interface}. */ + private static final Set<String> DECLARATION_KEYWORDS = new HashSet<>( + Arrays.asList("class", "interface", "enum", "record")); + + /** + * The reserved java keywords, which can never be a type name. They are dropped from the mentioned + * identifiers to keep the index small and to avoid matching the keywords that would otherwise + * appear in every file. Contextual keywords such as {@code record} or {@code var} are deliberately + * not listed, they can be type names. + */ + private static final Set<String> RESERVED_KEYWORDS = new HashSet<>(Arrays.asList( + "abstract", "assert", "boolean", "break", "byte", "case", "catch", "char", "class", "const", + "continue", "default", "do", "double", "else", "enum", "extends", "final", "finally", "float", + "for", "goto", "if", "implements", "import", "instanceof", "int", "interface", "long", "native", + "new", "package", "private", "protected", "public", "return", "short", "static", "strictfp", + "super", "switch", "synchronized", "this", "throw", "throws", "transient", "try", "void", + "volatile", "while", "true", "false", "null", "_")); + + /** entry point, see the {@code fast-fe} profile of fe-core/pom.xml. */ + public static void main(String[] args) throws Exception { + long startNanos = System.nanoTime(); + Map<String, String> options = parseOptions(args); + List<File> sourceRoots = splitPaths(requireOption(options, "sources")); + File touchRoot = new File(requireOption(options, "touch-root")).getCanonicalFile(); + File cacheFile = new File(requireOption(options, "cache")); + File classesDir = options.containsKey("classes") + ? new File(options.get("classes")).getCanonicalFile() : null; + + List<File> sourceFiles = findJavaFiles(sourceRoots); + SourceIndex index = SourceIndex.load(cacheFile); + + Map<String, Facts> factsByPath = new LinkedHashMap<>(); + List<Facts> derivedSources = new ArrayList<>(); + Map<String, List<String>> declaredBy = new TreeMap<>(); + Map<String, List<String>> referrersBy = new TreeMap<>(); + Set<String> changed = new TreeSet<>(); + int scanned = 0; + for (File sourceFile : sourceFiles) { + String path = sourceFile.getAbsolutePath(); + byte[] contentHash = contentHash(sourceFile); + Facts facts = index.get(path, contentHash); + if (facts == null) { + facts = scan(sourceFile, contentHash); + index.put(path, facts); + changed.add(path); + scanned++; + } + factsByPath.put(path, facts); + if (!isUnder(sourceFile, touchRoot)) { + derivedSources.add(facts); + } + for (String declaredName : facts.declaredNames) { + declaredBy.computeIfAbsent(declaredName, name -> new ArrayList<>()).add(path); + } + for (String identifier : facts.identifiers) { + referrersBy.computeIfAbsent(identifier, name -> new ArrayList<>()).add(path); + } + } + + // A file which disappeared, or a type which is not declared by the same files as before + // (added, removed, renamed, moved to another package), invalidates the files which mention + // that name exactly like a content change does. A removed file itself has nothing left to + // compile, only its classes have to go away, and its type names are already covered by the + // names whose declaring files changed. + Set<String> affected = new TreeSet<>(); + for (String staleName : index.replaceDeclaredNames(declaredBy)) { + addReferrers(affected, referrersBy, staleName); + } + for (String removedPath : index.removeMissing(sourceFiles)) { + removeClasses(new File(removedPath), touchRoot, classesDir); + } + affected.addAll(changed); + + // Propagate one hop only: the files which mention a type declared by a changed file. A file Review Comment: [P1] One-hop propagation is not safe for Java inheritance. For example, let `A` declare `public static final int VALUE = 2`, let `B extends A`, and let `C` return `B.VALUE`. After changing `A.VALUE` to 3, this loop marks only A and B because C mentions B but not A. Recompiling the marked sources leaves `C.class` with the old inlined value 2; I reproduced this and confirmed `iconst_2` with `javap`. A source that is recompiled without edits can expose a different effective API through its changed parent, so propagation must continue transitively unless ABI comparison proves that it can stop. ########## fe/fe-core/src/main/java/org/apache/doris/buildtools/IncrementalSourceMarker.java: ########## @@ -0,0 +1,588 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.buildtools; + +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.attribute.FileTime; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; + +/** + * Computes the set of sources which must be recompiled after a local edit and marks them for the + * compiler plugin, so that {@code mvn compile} stops rebuilding the whole fe-core module. + * + * <p>Background: {@code maven-compiler-plugin} is all-or-nothing by default (one stale source + * recompiles every source of the module), and its {@code useIncrementalCompilation=false} mode, + * which does compile only the stale sources, is unsafe on its own because it never recompiles the + * classes which <em>use</em> a changed class. + * + * <p>This tool closes that gap. For every scanned source file it caches, keyed by the content hash: + * <ul> + * <li>the identifiers the file mentions, which over-approximates the types it depends on,</li> + * <li>the type names the file declares.</li> + * </ul> + * A run then computes + * <pre> + * changed = files whose content changed, plus files whose set of declared names changed + * (a type was added, removed, renamed or moved) + * affected = changed, closed over "mentions a type name declared by an affected file" + * </pre> + * and updates the last modification time of every affected file which belongs to the main sources. + * The compiler plugin then compiles exactly that set. Because every file which mentions a changed + * type is touched, no class is left referring to a type which was changed or deleted underneath it; + * the set is a conservative superset, so the only cost of an imprecise name match is a few extra + * files being recompiled. + * + * <p>The compiler plugin still owns the compilation, its annotation processing and its classpath. + * This tool only decides which sources are stale. + */ +public class IncrementalSourceMarker { + /** Bump it whenever the cached facts change, then the index is rebuilt from scratch. */ + private static final int FORMAT_VERSION = 1; + + /** The java keywords which introduce a type name. {@code interface} also covers {@code @interface}. */ + private static final Set<String> DECLARATION_KEYWORDS = new HashSet<>( + Arrays.asList("class", "interface", "enum", "record")); + + /** + * The reserved java keywords, which can never be a type name. They are dropped from the mentioned + * identifiers to keep the index small and to avoid matching the keywords that would otherwise + * appear in every file. Contextual keywords such as {@code record} or {@code var} are deliberately + * not listed, they can be type names. + */ + private static final Set<String> RESERVED_KEYWORDS = new HashSet<>(Arrays.asList( + "abstract", "assert", "boolean", "break", "byte", "case", "catch", "char", "class", "const", + "continue", "default", "do", "double", "else", "enum", "extends", "final", "finally", "float", + "for", "goto", "if", "implements", "import", "instanceof", "int", "interface", "long", "native", + "new", "package", "private", "protected", "public", "return", "short", "static", "strictfp", + "super", "switch", "synchronized", "this", "throw", "throws", "transient", "try", "void", + "volatile", "while", "true", "false", "null", "_")); + + /** entry point, see the {@code fast-fe} profile of fe-core/pom.xml. */ + public static void main(String[] args) throws Exception { + long startNanos = System.nanoTime(); + Map<String, String> options = parseOptions(args); + List<File> sourceRoots = splitPaths(requireOption(options, "sources")); + File touchRoot = new File(requireOption(options, "touch-root")).getCanonicalFile(); + File cacheFile = new File(requireOption(options, "cache")); + File classesDir = options.containsKey("classes") + ? new File(options.get("classes")).getCanonicalFile() : null; + + List<File> sourceFiles = findJavaFiles(sourceRoots); + SourceIndex index = SourceIndex.load(cacheFile); + + Map<String, Facts> factsByPath = new LinkedHashMap<>(); + List<Facts> derivedSources = new ArrayList<>(); + Map<String, List<String>> declaredBy = new TreeMap<>(); + Map<String, List<String>> referrersBy = new TreeMap<>(); + Set<String> changed = new TreeSet<>(); + int scanned = 0; + for (File sourceFile : sourceFiles) { + String path = sourceFile.getAbsolutePath(); + byte[] contentHash = contentHash(sourceFile); + Facts facts = index.get(path, contentHash); + if (facts == null) { + facts = scan(sourceFile, contentHash); + index.put(path, facts); + changed.add(path); + scanned++; + } + factsByPath.put(path, facts); + if (!isUnder(sourceFile, touchRoot)) { + derivedSources.add(facts); + } + for (String declaredName : facts.declaredNames) { + declaredBy.computeIfAbsent(declaredName, name -> new ArrayList<>()).add(path); + } + for (String identifier : facts.identifiers) { + referrersBy.computeIfAbsent(identifier, name -> new ArrayList<>()).add(path); + } + } + + // A file which disappeared, or a type which is not declared by the same files as before + // (added, removed, renamed, moved to another package), invalidates the files which mention + // that name exactly like a content change does. A removed file itself has nothing left to + // compile, only its classes have to go away, and its type names are already covered by the + // names whose declaring files changed. + Set<String> affected = new TreeSet<>(); + for (String staleName : index.replaceDeclaredNames(declaredBy)) { + addReferrers(affected, referrersBy, staleName); + } + for (String removedPath : index.removeMissing(sourceFiles)) { + removeClasses(new File(removedPath), touchRoot, classesDir); + } + affected.addAll(changed); + + // Propagate one hop only: the files which mention a type declared by a changed file. A file + // which is recompiled without being edited keeps its own api, so there is no reason to go + // further, and a transitive closure would quickly reach the whole module. + // + // The exception is a source derived from another one, such as the Immutables output of an + // annotated class: it is regenerated when its input changes, so its api may change too, and + // the files which use it need a recompilation as well. + for (String path : changed) { + Facts facts = factsByPath.get(path); + if (facts == null) { + continue; + } + for (String declaredName : facts.declaredNames) { + addReferrers(affected, referrersBy, declaredName); + for (Facts derived : derivedSources) { + if (derived.identifiers.contains(declaredName)) { + for (String derivedName : derived.declaredNames) { + addReferrers(affected, referrersBy, derivedName); + } + } + } + } + } + + int marked = 0; + FileTime now = FileTime.fromMillis(System.currentTimeMillis()); + for (String path : affected) { + File file = new File(path); + if (isUnder(file, touchRoot)) { + Files.setLastModifiedTime(file.toPath(), now); + marked++; + } + } + + index.save(cacheFile); + long elapsedMillis = (System.nanoTime() - startNanos) / 1_000_000; + System.out.println("[incremental-sources] " + sourceFiles.size() + " sources, " + scanned + + " scanned, " + changed.size() + " changed, " + marked + " marked for recompilation in " + + elapsedMillis + " ms"); + } + + private static void addReferrers(Set<String> affected, Map<String, List<String>> referrersBy, String name) { + List<String> referrers = referrersBy.get(name); + if (referrers != null) { + affected.addAll(referrers); + } + } + + /** delete the classes of a source file which does not exist anymore. */ + private static void removeClasses(File removedSource, File touchRoot, File classesDir) throws IOException { + if (classesDir == null || !isUnder(removedSource, touchRoot)) { + return; + } + String relative = touchRoot.toPath().relativize(removedSource.getCanonicalFile().toPath()).toString(); + String prefix = relative.substring(0, relative.length() - ".java".length()); + File parent = new File(classesDir, prefix).getParentFile(); + if (parent == null || !parent.isDirectory()) { + return; + } + String simpleName = new File(prefix).getName(); + File[] candidates = parent.listFiles((dir, name) -> name.endsWith(".class") + && (name.equals(simpleName + ".class") || name.startsWith(simpleName + "$"))); + if (candidates != null) { + for (File candidate : candidates) { + Files.deleteIfExists(candidate.toPath()); + } + } + } + + private static boolean isUnder(File file, File directory) { Review Comment: [P1] `touchRoot` is canonicalized, but scanned files and cache keys use absolute paths. These differ whenever the checkout/root contains a symlink; on macOS the included test already triggers this because `mktemp` returns `/var/folders/...` while the canonical path is `/private/var/folders/...`. `isUnder` then returns false for every source, so nothing is touched and removed classes are not cleaned. Running `test-incremental-source-marker.sh` on macOS produced four failures for exactly this reason. Please normalize all roots/files/cache keys consistently and use `Path.startsWith` rather than string-prefix comparison. ########## fe/fe-core/src/main/java/org/apache/doris/buildtools/IncrementalSourceMarker.java: ########## @@ -0,0 +1,588 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.buildtools; + +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.attribute.FileTime; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; + +/** + * Computes the set of sources which must be recompiled after a local edit and marks them for the + * compiler plugin, so that {@code mvn compile} stops rebuilding the whole fe-core module. + * + * <p>Background: {@code maven-compiler-plugin} is all-or-nothing by default (one stale source + * recompiles every source of the module), and its {@code useIncrementalCompilation=false} mode, + * which does compile only the stale sources, is unsafe on its own because it never recompiles the + * classes which <em>use</em> a changed class. + * + * <p>This tool closes that gap. For every scanned source file it caches, keyed by the content hash: + * <ul> + * <li>the identifiers the file mentions, which over-approximates the types it depends on,</li> + * <li>the type names the file declares.</li> + * </ul> + * A run then computes + * <pre> + * changed = files whose content changed, plus files whose set of declared names changed + * (a type was added, removed, renamed or moved) + * affected = changed, closed over "mentions a type name declared by an affected file" + * </pre> + * and updates the last modification time of every affected file which belongs to the main sources. + * The compiler plugin then compiles exactly that set. Because every file which mentions a changed + * type is touched, no class is left referring to a type which was changed or deleted underneath it; + * the set is a conservative superset, so the only cost of an imprecise name match is a few extra + * files being recompiled. + * + * <p>The compiler plugin still owns the compilation, its annotation processing and its classpath. + * This tool only decides which sources are stale. + */ +public class IncrementalSourceMarker { + /** Bump it whenever the cached facts change, then the index is rebuilt from scratch. */ + private static final int FORMAT_VERSION = 1; + + /** The java keywords which introduce a type name. {@code interface} also covers {@code @interface}. */ + private static final Set<String> DECLARATION_KEYWORDS = new HashSet<>( + Arrays.asList("class", "interface", "enum", "record")); + + /** + * The reserved java keywords, which can never be a type name. They are dropped from the mentioned + * identifiers to keep the index small and to avoid matching the keywords that would otherwise + * appear in every file. Contextual keywords such as {@code record} or {@code var} are deliberately + * not listed, they can be type names. + */ + private static final Set<String> RESERVED_KEYWORDS = new HashSet<>(Arrays.asList( + "abstract", "assert", "boolean", "break", "byte", "case", "catch", "char", "class", "const", + "continue", "default", "do", "double", "else", "enum", "extends", "final", "finally", "float", + "for", "goto", "if", "implements", "import", "instanceof", "int", "interface", "long", "native", + "new", "package", "private", "protected", "public", "return", "short", "static", "strictfp", + "super", "switch", "synchronized", "this", "throw", "throws", "transient", "try", "void", + "volatile", "while", "true", "false", "null", "_")); + + /** entry point, see the {@code fast-fe} profile of fe-core/pom.xml. */ + public static void main(String[] args) throws Exception { + long startNanos = System.nanoTime(); + Map<String, String> options = parseOptions(args); + List<File> sourceRoots = splitPaths(requireOption(options, "sources")); + File touchRoot = new File(requireOption(options, "touch-root")).getCanonicalFile(); + File cacheFile = new File(requireOption(options, "cache")); + File classesDir = options.containsKey("classes") + ? new File(options.get("classes")).getCanonicalFile() : null; + + List<File> sourceFiles = findJavaFiles(sourceRoots); + SourceIndex index = SourceIndex.load(cacheFile); + + Map<String, Facts> factsByPath = new LinkedHashMap<>(); + List<Facts> derivedSources = new ArrayList<>(); + Map<String, List<String>> declaredBy = new TreeMap<>(); + Map<String, List<String>> referrersBy = new TreeMap<>(); + Set<String> changed = new TreeSet<>(); + int scanned = 0; + for (File sourceFile : sourceFiles) { + String path = sourceFile.getAbsolutePath(); + byte[] contentHash = contentHash(sourceFile); + Facts facts = index.get(path, contentHash); + if (facts == null) { + facts = scan(sourceFile, contentHash); + index.put(path, facts); + changed.add(path); + scanned++; + } + factsByPath.put(path, facts); + if (!isUnder(sourceFile, touchRoot)) { + derivedSources.add(facts); + } + for (String declaredName : facts.declaredNames) { + declaredBy.computeIfAbsent(declaredName, name -> new ArrayList<>()).add(path); + } + for (String identifier : facts.identifiers) { + referrersBy.computeIfAbsent(identifier, name -> new ArrayList<>()).add(path); + } + } + + // A file which disappeared, or a type which is not declared by the same files as before + // (added, removed, renamed, moved to another package), invalidates the files which mention + // that name exactly like a content change does. A removed file itself has nothing left to + // compile, only its classes have to go away, and its type names are already covered by the + // names whose declaring files changed. + Set<String> affected = new TreeSet<>(); + for (String staleName : index.replaceDeclaredNames(declaredBy)) { + addReferrers(affected, referrersBy, staleName); + } + for (String removedPath : index.removeMissing(sourceFiles)) { + removeClasses(new File(removedPath), touchRoot, classesDir); + } + affected.addAll(changed); + + // Propagate one hop only: the files which mention a type declared by a changed file. A file + // which is recompiled without being edited keeps its own api, so there is no reason to go + // further, and a transitive closure would quickly reach the whole module. + // + // The exception is a source derived from another one, such as the Immutables output of an + // annotated class: it is regenerated when its input changes, so its api may change too, and + // the files which use it need a recompilation as well. + for (String path : changed) { + Facts facts = factsByPath.get(path); + if (facts == null) { + continue; + } + for (String declaredName : facts.declaredNames) { + addReferrers(affected, referrersBy, declaredName); + for (Facts derived : derivedSources) { + if (derived.identifiers.contains(declaredName)) { + for (String derivedName : derived.declaredNames) { + addReferrers(affected, referrersBy, derivedName); + } + } + } + } + } + + int marked = 0; + FileTime now = FileTime.fromMillis(System.currentTimeMillis()); + for (String path : affected) { + File file = new File(path); + if (isUnder(file, touchRoot)) { + Files.setLastModifiedTime(file.toPath(), now); + marked++; + } + } + + index.save(cacheFile); + long elapsedMillis = (System.nanoTime() - startNanos) / 1_000_000; + System.out.println("[incremental-sources] " + sourceFiles.size() + " sources, " + scanned + + " scanned, " + changed.size() + " changed, " + marked + " marked for recompilation in " + + elapsedMillis + " ms"); + } + + private static void addReferrers(Set<String> affected, Map<String, List<String>> referrersBy, String name) { + List<String> referrers = referrersBy.get(name); + if (referrers != null) { + affected.addAll(referrers); + } + } + + /** delete the classes of a source file which does not exist anymore. */ + private static void removeClasses(File removedSource, File touchRoot, File classesDir) throws IOException { + if (classesDir == null || !isUnder(removedSource, touchRoot)) { + return; + } + String relative = touchRoot.toPath().relativize(removedSource.getCanonicalFile().toPath()).toString(); + String prefix = relative.substring(0, relative.length() - ".java".length()); + File parent = new File(classesDir, prefix).getParentFile(); + if (parent == null || !parent.isDirectory()) { + return; + } + String simpleName = new File(prefix).getName(); Review Comment: [P2] Class cleanup assumes every binary name starts with the source filename. A legal `Removed.java` containing `public class Removed { static class Inner {} }` plus package-private top-level `class Extra {}` produces `Removed.class`, `Removed$Inner.class`, and `Extra.class`; after deleting the source, this code removes the first two but leaves `Extra.class`. I reproduced that stale class. Since the index already records all `declaredNames`, cleanup should remove every top-level type declared by the removed source and each type's `$` classes; please add this multi-top-level-class case to the test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
