From 526316ced014070997e63d8cb64fd6f15f522c9c Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Mon, 5 Aug 2024 16:11:05 +0100 Subject: [PATCH 1/9] Fix exception reporting in javac plugin Without closing the print writer, empty message was produced. --- .../sourcegraph/semanticdb_javac/SemanticdbTaskListener.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java index 3844d0b3..f2594c76 100644 --- a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java +++ b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java @@ -83,7 +83,9 @@ public void finished(TaskEvent e) { // exception, it just prints the location with an empty message. private void reportException(Throwable exception, TaskEvent e) { ByteArrayOutputStream baos = new ByteArrayOutputStream(); - exception.printStackTrace(new PrintWriter(baos)); + PrintWriter pw = new PrintWriter(baos); + exception.printStackTrace(pw); + pw.close(); reporter.error(baos.toString(), e.getCompilationUnit(), e.getCompilationUnit()); } From f8f1dc0b2cd1ca44150131ebceeb0ce1468f3d61 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Mon, 5 Aug 2024 16:13:48 +0100 Subject: [PATCH 2/9] Add a test reproducing customer issue --- .../scala/tests/MavenBuildToolSuite.scala | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/buildTools/src/test/scala/tests/MavenBuildToolSuite.scala b/tests/buildTools/src/test/scala/tests/MavenBuildToolSuite.scala index 93f984cc..1ddba612 100644 --- a/tests/buildTools/src/test/scala/tests/MavenBuildToolSuite.scala +++ b/tests/buildTools/src/test/scala/tests/MavenBuildToolSuite.scala @@ -32,6 +32,26 @@ class MavenBuildToolSuite extends BaseBuildToolSuite { |""".stripMargin ) + // TODO: find more of a core location to move this to + checkBuild( + options = "annotation-parameters", + original = + s""" + |/pom.xml + |$pomXml + |/src/main/java/com/Foo.java + |public interface Foo { + | @Bar(-1d) + | double value(); + |} + |/src/main/java/com/Bar.java + |@interface Bar { + | double value(); + |} + |""".stripMargin, + expectedSemanticdbFiles = 2 + ) + checkBuild( "build-command", s"""|/pom.xml From e4128b7ba49e9b6aab95e45b69ca672f1e3d9c35 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Mon, 5 Aug 2024 16:16:35 +0100 Subject: [PATCH 3/9] (root cause) add unary tree to semanticdb and handle it --- .../semanticdb_javac/SemanticdbBuilders.java | 9 ++++++++ .../src/main/protobuf/semanticdb.proto | 17 +++++++++++++++ .../semanticdb_javac/SemanticdbTrees.java | 21 ++++++++++++++++++- 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbBuilders.java b/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbBuilders.java index 492572a1..da11befd 100644 --- a/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbBuilders.java +++ b/semanticdb-java/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbBuilders.java @@ -146,6 +146,15 @@ public static Semanticdb.BinaryOperatorTree binopTree( .build(); } + public static Semanticdb.Tree tree(Semanticdb.UnaryOperatorTree unaryOperatorTree) { + return Semanticdb.Tree.newBuilder().setUnaryopTree(unaryOperatorTree).build(); + } + + public static Semanticdb.UnaryOperatorTree unaryOpTree( + Semanticdb.UnaryOperator operator, Semanticdb.Tree rhs) { + return Semanticdb.UnaryOperatorTree.newBuilder().setOp(operator).setTree(rhs).build(); + } + public static Semanticdb.Tree tree(Semanticdb.AssignTree assignTree) { return Semanticdb.Tree.newBuilder().setAssignTree(assignTree).build(); } diff --git a/semanticdb-java/src/main/protobuf/semanticdb.proto b/semanticdb-java/src/main/protobuf/semanticdb.proto index 99857524..78353595 100644 --- a/semanticdb-java/src/main/protobuf/semanticdb.proto +++ b/semanticdb-java/src/main/protobuf/semanticdb.proto @@ -297,6 +297,7 @@ message Tree { AnnotationTree annotation_tree = 9; AssignTree assign_tree = 10; BinaryOperatorTree binop_tree = 11; + UnaryOperatorTree unaryop_tree = 12; // -- OUT OF SPEC -- // } } @@ -378,6 +379,22 @@ enum BinaryOperator { GREATER_THAN_EQUAL = 17; LESS_THAN_EQUAL = 18; } + +message UnaryOperatorTree { + UnaryOperator op = 1; + Tree tree = 2; +} + +enum UnaryOperator { + UNARY_MINUS = 0; + UNARY_PLUS = 1; + UNARY_POSTFIX_INCREMENT = 2; + UNARY_POSTFIX_DECREMENT = 3; + UNARY_PREFIX_DECREMENT = 4; + UNARY_PREFIX_INCREMENT = 5; + UNARY_BITWISE_COMPLEMENT = 6; + UNARY_LOGICAL_COMPLEMENT = 7; +} // -- OUT OF SPEC -- // message Constant { diff --git a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTrees.java b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTrees.java index 93c0bd60..e0aa70d9 100644 --- a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTrees.java +++ b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTrees.java @@ -8,6 +8,7 @@ import com.sun.source.tree.Tree; import com.sun.source.tree.Tree.Kind; import com.sun.source.tree.BinaryTree; +import com.sun.source.tree.UnaryTree; import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.MemberSelectTree; import com.sun.source.tree.ClassTree; @@ -78,7 +79,8 @@ public Semanticdb.AnnotationTree annotationBuilder(AnnotationTree annotation) { ArrayList params = new ArrayList<>(annotation.getArguments().size()); for (ExpressionTree param : annotation.getArguments()) { - // anecdotally not always AssignmentTree in some situations when a compilation unit can't + // anecdotally not always AssignmentTree in some situations when a compilation + // unit can't // resolve symbols fully if (param instanceof AssignmentTree) { AssignmentTree assign = (AssignmentTree) param; @@ -152,6 +154,12 @@ private Semanticdb.Tree annotationParameter(ExpressionTree expr) { annotationParameter(binExpr.getLeftOperand()), semanticdbBinaryOperator(expr.getKind()), annotationParameter(binExpr.getRightOperand()))); + } else if (expr instanceof UnaryTree) { + UnaryTree unaryExpr = (UnaryTree) expr; + return tree( + unaryOpTree( + semanticdbUnaryOperator(unaryExpr.getKind()), + annotationParameter(unaryExpr.getExpression()))); } throw new IllegalArgumentException( semanticdbUri @@ -206,4 +214,15 @@ private Semanticdb.BinaryOperator semanticdbBinaryOperator(Tree.Kind kind) { semanticdbUri + ": unexpected binary expression operator kind " + kind); } } + + private Semanticdb.UnaryOperator semanticdbUnaryOperator(Tree.Kind kind) { + switch (kind) { + case UNARY_MINUS: + return Semanticdb.UnaryOperator.UNARY_MINUS; + + default: + throw new IllegalStateException( + semanticdbUri + ": unexpected unary expression operator kind " + kind); + } + } } From 15ff2cd9baeac705ac6fb58a57b7ee3cb0f18279 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Mon, 5 Aug 2024 16:26:55 +0100 Subject: [PATCH 4/9] Fix unary operation support to SignatureFormatter --- .../scip_semanticdb/SignatureFormatter.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/SignatureFormatter.java b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/SignatureFormatter.java index ce113391..aec78d52 100644 --- a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/SignatureFormatter.java +++ b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/SignatureFormatter.java @@ -468,11 +468,39 @@ private String formatTree(Tree tree) { return formatTree(tree.getAssignTree().getLhs()) + " = " + formatTree(tree.getAssignTree().getRhs()); + } else if (tree.hasUnaryopTree()) { + return formatUnaryOperation(tree.getUnaryopTree()); } throw new IllegalArgumentException("tree was of unexpected type " + tree); } + private String formatUnaryOperation(UnaryOperatorTree tree) { + String formattedValue = formatTree(tree.getTree()); + switch (tree.getOp()) { + case UNARY_MINUS: + return "-" + formattedValue; + case UNARY_PLUS: + return "-" + formattedValue; + case UNARY_POSTFIX_INCREMENT: + return formattedValue + "++"; + case UNARY_POSTFIX_DECREMENT: + return formattedValue + "--"; + case UNARY_PREFIX_DECREMENT: + return "--" + formattedValue; + case UNARY_PREFIX_INCREMENT: + return "++" + formattedValue; + + case UNARY_BITWISE_COMPLEMENT: + return "~" + formattedValue; + case UNARY_LOGICAL_COMPLEMENT: + return "!" + formattedValue; + } + + throw new IllegalArgumentException( + "unary operation of unexpected type" + tree.getOp().toString()); + } + private String formatConstant(Constant constant) { if (constant.hasUnitConstant()) { return isScala ? "()" : "scala.Unit()"; From 7ee19e2264b6604119c80fd74ddfbad3593011d1 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Mon, 5 Aug 2024 16:39:54 +0100 Subject: [PATCH 5/9] Move tests to snapshots --- .../semanticdb_javac/SemanticdbTrees.java | 21 ++++++++++ .../scala/tests/MavenBuildToolSuite.scala | 20 ---------- .../java/minimized/AnnotationParameters.java | 20 ++++++++++ .../java/minimized/AnnotationParameters.java | 38 +++++++++++++++++++ 4 files changed, 79 insertions(+), 20 deletions(-) create mode 100644 tests/minimized/src/main/java/minimized/AnnotationParameters.java create mode 100644 tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java diff --git a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTrees.java b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTrees.java index e0aa70d9..90e5889d 100644 --- a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTrees.java +++ b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTrees.java @@ -220,6 +220,27 @@ private Semanticdb.UnaryOperator semanticdbUnaryOperator(Tree.Kind kind) { case UNARY_MINUS: return Semanticdb.UnaryOperator.UNARY_MINUS; + case UNARY_PLUS: + return Semanticdb.UnaryOperator.UNARY_PLUS; + + case POSTFIX_INCREMENT: + return Semanticdb.UnaryOperator.UNARY_POSTFIX_INCREMENT; + + case POSTFIX_DECREMENT: + return Semanticdb.UnaryOperator.UNARY_POSTFIX_DECREMENT; + + case PREFIX_INCREMENT: + return Semanticdb.UnaryOperator.UNARY_PREFIX_INCREMENT; + + case PREFIX_DECREMENT: + return Semanticdb.UnaryOperator.UNARY_PREFIX_DECREMENT; + + case BITWISE_COMPLEMENT: + return Semanticdb.UnaryOperator.UNARY_BITWISE_COMPLEMENT; + + case LOGICAL_COMPLEMENT: + return Semanticdb.UnaryOperator.UNARY_LOGICAL_COMPLEMENT; + default: throw new IllegalStateException( semanticdbUri + ": unexpected unary expression operator kind " + kind); diff --git a/tests/buildTools/src/test/scala/tests/MavenBuildToolSuite.scala b/tests/buildTools/src/test/scala/tests/MavenBuildToolSuite.scala index 1ddba612..93f984cc 100644 --- a/tests/buildTools/src/test/scala/tests/MavenBuildToolSuite.scala +++ b/tests/buildTools/src/test/scala/tests/MavenBuildToolSuite.scala @@ -32,26 +32,6 @@ class MavenBuildToolSuite extends BaseBuildToolSuite { |""".stripMargin ) - // TODO: find more of a core location to move this to - checkBuild( - options = "annotation-parameters", - original = - s""" - |/pom.xml - |$pomXml - |/src/main/java/com/Foo.java - |public interface Foo { - | @Bar(-1d) - | double value(); - |} - |/src/main/java/com/Bar.java - |@interface Bar { - | double value(); - |} - |""".stripMargin, - expectedSemanticdbFiles = 2 - ) - checkBuild( "build-command", s"""|/pom.xml diff --git a/tests/minimized/src/main/java/minimized/AnnotationParameters.java b/tests/minimized/src/main/java/minimized/AnnotationParameters.java new file mode 100644 index 00000000..a8517d88 --- /dev/null +++ b/tests/minimized/src/main/java/minimized/AnnotationParameters.java @@ -0,0 +1,20 @@ +package minimized; + +interface Foo { + @Bar(-1d) + double test(); + + @Bar(~5) + double test2(); + + @BarB(!true) + double test3(); +} + +@interface Bar { + double value(); +} + +@interface BarB { + boolean value(); +} diff --git a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java new file mode 100644 index 00000000..5d9d38d3 --- /dev/null +++ b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java @@ -0,0 +1,38 @@ +package minimized; + +interface Foo { +// ^^^ definition semanticdb maven . . minimized/Foo# +// display_name Foo +// signature_documentation java interface Foo +// kind Interface + @Bar(-1d) + double test(); + + @Bar(~5) + double test2(); + + @BarB(!true) + double test3(); +} + +@interface Bar { +// ^^^ definition semanticdb maven . . minimized/Bar# +// display_name Bar +// signature_documentation java @interface Bar +// kind Interface +// relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation# + double value(); +} + +@interface BarB { +// ^^^^ definition semanticdb maven . . minimized/BarB# +// display_name BarB +// signature_documentation java @interface BarB +// kind Interface +// relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation# + boolean value(); +// ^^^^^ definition semanticdb maven . . minimized/BarB#value(). +// display_name value +// signature_documentation java public abstract boolean value() +// kind AbstractMethod +} From a152862c639c7643758458548847c1e6e4bf8304 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Mon, 5 Aug 2024 16:43:48 +0100 Subject: [PATCH 6/9] Check if we get method annotation references on built-in ones.. --- .../minimized/src/main/java/minimized/AnnotationParameters.java | 1 + .../minimized/src/main/java/minimized/AnnotationParameters.java | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/minimized/src/main/java/minimized/AnnotationParameters.java b/tests/minimized/src/main/java/minimized/AnnotationParameters.java index a8517d88..4eb948b1 100644 --- a/tests/minimized/src/main/java/minimized/AnnotationParameters.java +++ b/tests/minimized/src/main/java/minimized/AnnotationParameters.java @@ -5,6 +5,7 @@ interface Foo { double test(); @Bar(~5) + @SuppressWarnings(value = "unchecked") double test2(); @BarB(!true) diff --git a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java index 5d9d38d3..98ba27a3 100644 --- a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java +++ b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java @@ -9,6 +9,7 @@ interface Foo { double test(); @Bar(~5) + @SuppressWarnings(value = "unchecked") double test2(); @BarB(!true) From 115d6ce1259c61682047d53cb2679d75700da557 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 6 Aug 2024 10:09:46 +0100 Subject: [PATCH 7/9] Move snapshot tests --- .../java/minimized/AnnotationParameters.java | 16 +++--- .../java/minimized/AnnotationParameters.java | 53 ++++++++++++------- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/tests/minimized/src/main/java/minimized/AnnotationParameters.java b/tests/minimized/src/main/java/minimized/AnnotationParameters.java index 4eb948b1..d2049d3f 100644 --- a/tests/minimized/src/main/java/minimized/AnnotationParameters.java +++ b/tests/minimized/src/main/java/minimized/AnnotationParameters.java @@ -1,5 +1,14 @@ package minimized; + +@interface Bar { + double value(); +} + +@interface BarB { + boolean value(); +} + interface Foo { @Bar(-1d) double test(); @@ -12,10 +21,3 @@ interface Foo { double test3(); } -@interface Bar { - double value(); -} - -@interface BarB { - boolean value(); -} diff --git a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java index 98ba27a3..fbb0fbf3 100644 --- a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java +++ b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java @@ -1,20 +1,5 @@ package minimized; -interface Foo { -// ^^^ definition semanticdb maven . . minimized/Foo# -// display_name Foo -// signature_documentation java interface Foo -// kind Interface - @Bar(-1d) - double test(); - - @Bar(~5) - @SuppressWarnings(value = "unchecked") - double test2(); - - @BarB(!true) - double test3(); -} @interface Bar { // ^^^ definition semanticdb maven . . minimized/Bar# @@ -32,8 +17,38 @@ interface Foo { // kind Interface // relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation# boolean value(); -// ^^^^^ definition semanticdb maven . . minimized/BarB#value(). -// display_name value -// signature_documentation java public abstract boolean value() -// kind AbstractMethod } + +interface Foo { +// ^^^ definition semanticdb maven . . minimized/Foo# +// display_name Foo +// signature_documentation java interface Foo +// kind Interface + @Bar(-1d) +// ^^^ reference semanticdb maven . . minimized/Bar# + double test(); +// ^^^^ definition semanticdb maven . . minimized/Foo#test(). +// display_name test +// signature_documentation java @Bar(-1.0)\npublic abstract double test() +// kind AbstractMethod + + @Bar(~5) +// ^^^ reference semanticdb maven . . minimized/Bar# + @SuppressWarnings(value = "unchecked") +// ^^^^^^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/SuppressWarnings# +// ^^^^^ reference semanticdb maven jdk 11 java/lang/SuppressWarnings#value(). + double test2(); +// ^^^^^ definition semanticdb maven . . minimized/Foo#test2(). +// display_name test2 +// signature_documentation java @Bar(~5)\n@SuppressWarnings("unchecked")\npublic abstract double test2() +// kind AbstractMethod + + @BarB(!true) +// ^^^^ reference semanticdb maven . . minimized/BarB# + double test3(); +// ^^^^^ definition semanticdb maven . . minimized/Foo#test3(). +// display_name test3 +// signature_documentation java @BarB(!true)\npublic abstract double test3() +// kind AbstractMethod +} + From f8f12f658a31d0a9127de2af90217864d4a0af95 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 6 Aug 2024 10:09:59 +0100 Subject: [PATCH 8/9] Allow regenerating a subset of snapshots --- .../src/main/scala/tests/SaveSnapshots.scala | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/snapshots/src/main/scala/tests/SaveSnapshots.scala b/tests/snapshots/src/main/scala/tests/SaveSnapshots.scala index 2859051f..d489fb7e 100644 --- a/tests/snapshots/src/main/scala/tests/SaveSnapshots.scala +++ b/tests/snapshots/src/main/scala/tests/SaveSnapshots.scala @@ -3,7 +3,19 @@ package tests object SaveSnapshots { def main(args: Array[String]): Unit = { val expectDirectory = tests.snapshots.BuildInfo.snapshotDirectory.toPath - SemanticdbJavacSnapshotGenerator - .run(SnapshotContext(expectDirectory), new SaveSnapshotHandler) + val mapping = Map( + "minimized" -> new MinimizedSnapshotScipGenerator(), + "library" -> new LibrarySnapshotGenerator() + ) + + val enabledGenerators = + if (args.isEmpty) + mapping.values.toList + else + args.flatMap(mapping.get).toList + + val generator = new AggregateSnapshotGenerator(enabledGenerators) + + generator.run(SnapshotContext(expectDirectory), new SaveSnapshotHandler) } } From d3860892931f25af5da129a5d23c8e54b2343f72 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 6 Aug 2024 10:33:00 +0100 Subject: [PATCH 9/9] Add link to JLS --- semanticdb-java/src/main/protobuf/semanticdb.proto | 1 + 1 file changed, 1 insertion(+) diff --git a/semanticdb-java/src/main/protobuf/semanticdb.proto b/semanticdb-java/src/main/protobuf/semanticdb.proto index 78353595..5f5ec5b4 100644 --- a/semanticdb-java/src/main/protobuf/semanticdb.proto +++ b/semanticdb-java/src/main/protobuf/semanticdb.proto @@ -380,6 +380,7 @@ enum BinaryOperator { LESS_THAN_EQUAL = 18; } +// https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.15 message UnaryOperatorTree { UnaryOperator op = 1; Tree tree = 2;