Skip to content

Commit

Permalink
Update the GwtIncompatibleStripper to support removing multiple annot…
Browse files Browse the repository at this point in the history 
…ations

PiperOrigin-RevId: 715018123
  • Loading branch information
@kevinoconnor7 @copybara-github
kevinoconnor7 authored and copybara-github committed Jan 13, 2025
1 parent e5c097f commit d37b917
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 39 deletions.
11 changes: 6 additions & 5 deletions build_defs/internal_do_not_use/j2cl_common.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ def _java_compile(
output_jar = None,
javac_opts = [],
mnemonic = "J2cl",
strip_annotation = "GwtIncompatible"):
strip_annotations = ["GwtIncompatible"]):
output_jar = output_jar or ctx.actions.declare_file("lib%s.jar" % name)
stripped_java_srcs = [_strip_incompatible_annotation(ctx, name, srcs, mnemonic, strip_annotation)] if srcs else []
stripped_java_srcs = [_strip_incompatible_annotation(ctx, name, srcs, mnemonic, strip_annotations)] if srcs else []
javac_opts = DEFAULT_J2CL_JAVAC_OPTS + javac_opts

if ctx.var.get("GROK_ELLIPSIS_BUILD", None):
Expand Down Expand Up @@ -240,19 +240,20 @@ def get_jdk_system(java_toolchain, javac_opts):
# TODO(b/197211878): Switch to a public API when available.
return java_toolchain._bootclasspath_info._system_inputs.to_list() if not jdk_system_already_set else []

def _strip_incompatible_annotation(ctx, name, java_srcs, mnemonic, strip_annotation):
def _strip_incompatible_annotation(ctx, name, java_srcs, mnemonic, strip_annotations):
# Paths are matched by Kythe to identify generated J2CL sources.
output_file = ctx.actions.declare_file(name + "_j2cl_stripped-src.jar")

args = ctx.actions.args()
args.use_param_file("@%s", use_always = True)
args.set_param_file_format("multiline")
args.add("-d", output_file)
args.add("-annotation", strip_annotation)
args.add_all(strip_annotations, format_each = "-annotation=%s")
args.add_all(java_srcs)

formatted_annotations = ", ".join(["@" + annotation for annotation in strip_annotations])
ctx.actions.run(
progress_message = "Stripping @%s from %s" % (strip_annotation, name),
progress_message = "Stripping %s from %s" % (formatted_annotations, name),
inputs = java_srcs,
outputs = [output_file],
executable = ctx.executable._j2cl_stripper,
Expand Down
2 changes: 1 addition & 1 deletion build_defs/internal_do_not_use/j2cl_java_library.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def _impl_j2cl_library(ctx):
exported_plugins = [p[JavaPluginInfo] for p in ctx.attr.exported_plugins],
output_jar = ctx.actions.declare_file(ctx.label.name + "_j2kt_web_jvm.jar"),
javac_opts = extra_javacopts + ctx.attr.javacopts,
strip_annotation = "GwtIncompatible",
strip_annotations = ["GwtIncompatible"],
# TODO(b/322906767): Remove when the bug is fixed.
custom_args = [
"--jvm_flag=-Dcom.google.j2cl.transpiler.backend.kotlin.preserveEqualsForJsTypeInterface=true",
Expand Down
9 changes: 6 additions & 3 deletions tools/java/com/google/j2cl/tools/gwtincompatible/BazelGwtIncompatibleStripper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@ final class BazelGwtIncompatibleStripper extends BazelWorker {
@Option(
name = "-annotation",
metaVar = "<annotation>",
usage = "The name of hte annotation to strip; defaults to 'GwtIncompatible'")
String annotation = "GwtIncompatible";
usage = "The name(s) of annotations to strip; defaults to 'GwtIncompatible'")
List<String> annotations = new ArrayList<>();

@Override
protected void run(Problems problems) {
GwtIncompatibleStripper.strip(files, outputPath, problems, annotation);
if (annotations.isEmpty()) {
annotations.add("GwtIncompatible");
}
GwtIncompatibleStripper.strip(files, outputPath, problems, annotations);
}

public static void main(String[] workerArgs) throws Exception {
Expand Down
34 changes: 22 additions & 12 deletions tools/java/com/google/j2cl/tools/gwtincompatible/GwtIncompatibleStripper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.dom.AST;
import org.eclipse.jdt.core.dom.ASTNode;
Expand All @@ -47,24 +50,25 @@
*/
public final class GwtIncompatibleStripper {

static void strip(List<String> files, Path outputPath, Problems problems, String annotationName) {
static void strip(
List<String> files, Path outputPath, Problems problems, List<String> annotationNames) {
try (Output out = OutputUtils.initOutput(outputPath, problems)) {
List<FileInfo> allPaths =
SourceUtils.getAllSources(files, problems)
.filter(f -> f.targetPath().endsWith(".java"))
.collect(toImmutableList());
preprocessFiles(allPaths, out, problems, annotationName);
preprocessFiles(allPaths, out, problems, annotationNames);
}
}

/** Preprocess all provided files and put them to provided output path. */
private static void preprocessFiles(
List<FileInfo> fileInfos, Output output, Problems problems, String annotationName) {
List<FileInfo> fileInfos, Output output, Problems problems, List<String> annotationNames) {
for (FileInfo fileInfo : fileInfos) {
String processedFileContent;
try {
String fileContent = MoreFiles.asCharSource(Paths.get(fileInfo.sourcePath()), UTF_8).read();
processedFileContent = strip(fileContent, annotationName);
processedFileContent = strip(fileContent, annotationNames);
} catch (IOException e) {
problems.fatal(FatalError.CANNOT_OPEN_FILE, e.toString());
return;
Expand All @@ -75,9 +79,9 @@ private static void preprocessFiles(
}
}

public static String strip(String fileContent, String annotationName) {
// Avoid parsing if there are no textual references to the annotation name.
if (!fileContent.contains(annotationName)) {
public static String strip(String fileContent, List<String> annotationNames) {
// Avoid parsing if there are no textual references to the annotation name(s).
if (annotationNames.stream().noneMatch(fileContent::contains)) {
return fileContent;
}

Expand All @@ -94,12 +98,15 @@ public static String strip(String fileContent, String annotationName) {
CompilationUnit compilationUnit = (CompilationUnit) parser.createAST(null);

// Find all the declarations with the annotation name
AnnotatedNodeCollector gwtIncompatibleVisitor = new AnnotatedNodeCollector(annotationName);
compilationUnit.accept(gwtIncompatibleVisitor);
List<ASTNode> gwtIncompatibleNodes = gwtIncompatibleVisitor.getNodes();
Set<ASTNode> nodesToRemove = new HashSet<>();
for (String annotationName : annotationNames) {
AnnotatedNodeCollector gwtIncompatibleVisitor = new AnnotatedNodeCollector(annotationName);
compilationUnit.accept(gwtIncompatibleVisitor);
nodesToRemove.addAll(gwtIncompatibleVisitor.getNodes());
}

// Delete the gwtIncompatible nodes.
for (ASTNode gwtIncompatibleNode : gwtIncompatibleNodes) {
for (ASTNode gwtIncompatibleNode : nodesToRemove) {
gwtIncompatibleNode.delete();
}

Expand All @@ -111,7 +118,10 @@ public static String strip(String fileContent, String annotationName) {
// Wrap all the not needed nodes inside comments in the original source
// (so we can preserve line numbers and have accurate source maps).
List<ASTNode> nodesToWrap = Lists.newArrayList(unusedImportsNodes);
nodesToWrap.addAll(gwtIncompatibleNodes);
// Add the nodes to remove in start position order.
nodesToRemove.stream()
.sorted(Comparator.comparingInt(ASTNode::getStartPosition))
.forEach(nodesToWrap::add);
if (nodesToWrap.isEmpty()) {
// Nothing was changed.
return fileContent;
Expand Down
9 changes: 6 additions & 3 deletions .../java/com/google/j2cl/tools/gwtincompatible/GwtIncompatibleStripperCommandLineRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,20 @@ public final class GwtIncompatibleStripperCommandLineRunner extends CommandLineT
@Option(
name = "-annotation",
metaVar = "<annotation>",
usage = "The name of the annotation to strip; defaults to 'GwtIncompatible'")
String annotation = "GwtIncompatible";
usage = "The name(s) of annotations to strip; defaults to 'GwtIncompatible'")
List<String> annotations = new ArrayList<>();

private GwtIncompatibleStripperCommandLineRunner() {
super("gwt-incompatible-stripper");
}

@Override
protected void run(Problems problems) {
if (annotations.isEmpty()) {
annotations.add("GwtIncompatible");
}
checkSourceFiles(problems, files, ".java", ".srcjar", ".jar");
GwtIncompatibleStripper.strip(files, outputPath, problems, annotation);
GwtIncompatibleStripper.strip(files, outputPath, problems, annotations);
}

public static int run(String[] args) {
Expand Down
72 changes: 57 additions & 15 deletions tools/javatests/com/google/j2cl/tools/gwtincompatible/GwtIncompatibleStripperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.junit.Assert.assertEquals;

import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -29,19 +30,22 @@ public class GwtIncompatibleStripperTest {
@Test
public void testNoProcess() {
String content = "public class Foo {}";
assertEquals(content, GwtIncompatibleStripper.strip(content, "GwtIncompatible"));
assertEquals(
content, GwtIncompatibleStripper.strip(content, ImmutableList.of("GwtIncompatible")));
}

@Test
public void testNoProcessOtherIncompatible() {
String content = "@J2kIncompatible\npublic class Foo {}";
assertEquals(content, GwtIncompatibleStripper.strip(content, "GwtIncompatible"));
assertEquals(
content, GwtIncompatibleStripper.strip(content, ImmutableList.of("GwtIncompatible")));
}

@Test
public void testNoProcessString() {
String content = "public class Foo {String a = \"@GwtIncompatible\");}";
assertEquals(content, GwtIncompatibleStripper.strip(content, "GwtIncompatible"));
assertEquals(
content, GwtIncompatibleStripper.strip(content, ImmutableList.of("GwtIncompatible")));
}

@Test
Expand All @@ -60,7 +64,7 @@ public void testProcessClass() {
stripped("public class Foo {"),
stripped(" public X m() {return null;}"),
stripped("}"));
assertEquals(after, GwtIncompatibleStripper.strip(before, "GwtIncompatible"));
assertEquals(after, GwtIncompatibleStripper.strip(before, ImmutableList.of("GwtIncompatible")));
}

@Test
Expand All @@ -79,11 +83,12 @@ public void testProcessJ2ktIncompatibleClass() {
stripped("public class Foo {"),
stripped(" public X m() {return null;}"),
stripped("}"));
assertEquals(after, GwtIncompatibleStripper.strip(before, "J2ktIncompatible"));
assertEquals(
after, GwtIncompatibleStripper.strip(before, ImmutableList.of("J2ktIncompatible")));
}

@Test
public void testProcessMultipleAnnotatons() {
public void testProcessMultipleAnnotationsPresent() {
String before =
lines(
"import a.b.X;",
Expand All @@ -100,7 +105,44 @@ public void testProcessMultipleAnnotatons() {
stripped("public class Foo {"),
stripped(" public X m() {return null;}"),
stripped("}"));
assertEquals(after, GwtIncompatibleStripper.strip(before, "J2ktIncompatible"));
assertEquals(
after, GwtIncompatibleStripper.strip(before, ImmutableList.of("J2ktIncompatible")));
}

@Test
public void testProcessMultipleAnnotationsToStrip() {
String before =
lines(
"import a.b.X;",
"import a.b.Y;",
"import a.b.Z;",
"public class Foo {",
" @GwtIncompatible",
" @J2ktIncompatible",
" public X m() {return null;}",
" @GwtIncompatible",
" public Y j2ktOnly() {return null;}",
" @J2ktIncompatible",
" public Z gwtOnly() {return null;}",
"}");
String after =
lines(
stripped("import a.b.X;"),
stripped("import a.b.Y;"),
stripped("import a.b.Z;"),
"public class Foo {",
stripped(" @GwtIncompatible"),
stripped(" @J2ktIncompatible"),
stripped(" public X m() {return null;}"),
stripped(" @GwtIncompatible"),
stripped(" public Y j2ktOnly() {return null;}"),
stripped(" @J2ktIncompatible"),
stripped(" public Z gwtOnly() {return null;}"),
"}");
assertEquals(
after,
GwtIncompatibleStripper.strip(
before, ImmutableList.of("J2ktIncompatible", "GwtIncompatible")));
}

@Test
Expand All @@ -125,7 +167,7 @@ public void testProcessMethod() {
stripped(" @GwtIncompatible"),
stripped(" public D n() {}"),
"}");
assertEquals(after, GwtIncompatibleStripper.strip(before, "GwtIncompatible"));
assertEquals(after, GwtIncompatibleStripper.strip(before, ImmutableList.of("GwtIncompatible")));
}

@Test
Expand All @@ -146,7 +188,7 @@ public void testProcessField() {
stripped(" public String b;"),
" public String c;",
"}");
assertEquals(after, GwtIncompatibleStripper.strip(before, "GwtIncompatible"));
assertEquals(after, GwtIncompatibleStripper.strip(before, ImmutableList.of("GwtIncompatible")));
}

@Test
Expand All @@ -160,7 +202,7 @@ public void testProcessEnumConstant() {
stripped(" B,"),
" C;",
"}");
assertEquals(after, GwtIncompatibleStripper.strip(before, "GwtIncompatible"));
assertEquals(after, GwtIncompatibleStripper.strip(before, ImmutableList.of("GwtIncompatible")));
}

@Test
Expand All @@ -179,7 +221,7 @@ public void testProcessAnnotationMember() {
stripped(" @GwtIncompatible"),
stripped(" public String n();"),
"}");
assertEquals(after, GwtIncompatibleStripper.strip(before, "GwtIncompatible"));
assertEquals(after, GwtIncompatibleStripper.strip(before, ImmutableList.of("GwtIncompatible")));
}

@Test
Expand Down Expand Up @@ -218,7 +260,7 @@ public void testProcessMultiple() {
stripped(" }"),
" String s;",
"}");
assertEquals(after, GwtIncompatibleStripper.strip(before, "GwtIncompatible"));
assertEquals(after, GwtIncompatibleStripper.strip(before, ImmutableList.of("GwtIncompatible")));
}

@Test
Expand All @@ -237,7 +279,7 @@ public void testNestedComment() {
stripped(" @GwtIncompatible"),
stripped(" public void n() {foo(x /* the value of x */);}"),
"}");
assertEquals(after, GwtIncompatibleStripper.strip(before, "GwtIncompatible"));
assertEquals(after, GwtIncompatibleStripper.strip(before, ImmutableList.of("GwtIncompatible")));
}

@Test
Expand All @@ -256,7 +298,7 @@ public void testTabs() {
stripped(" @GwtIncompatible"),
" \t" + stripped("public B n() {}"),
"}");
assertEquals(after, GwtIncompatibleStripper.strip(before, "GwtIncompatible"));
assertEquals(after, GwtIncompatibleStripper.strip(before, ImmutableList.of("GwtIncompatible")));
}

@Test
Expand All @@ -277,7 +319,7 @@ public void testMultibyteChars() {
stripped(" //மெ.பை."),
stripped(" public B n() {}"),
"}");
assertEquals(after, GwtIncompatibleStripper.strip(before, "GwtIncompatible"));
assertEquals(after, GwtIncompatibleStripper.strip(before, ImmutableList.of("GwtIncompatible")));
}

private static String lines(String... lines) {
Expand Down

0 comments on commit d37b917

Please sign in to comment.