git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v1 0/3] userdiff: Java updates
@ 2023-02-03 23:23 Andrei Rybak
  2023-02-03 23:23 ` [PATCH v1 1/3] userdiff: support Java type parameters Andrei Rybak
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-03 23:23 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Paolo Bonzini, Johannes Sixt, Tassilo Horn

Three patches to improve builtin userdiff support for Java features.  Some
pretty old features -- type parameters aka generics are actually older than Git,
and some quite recent features of Java: records and sealed classes, released in
Java 16 and Java 17 correspondingly, both from 2021.

https://en.wikipedia.org/wiki/Java_version_history

Andrei Rybak (3):
  userdiff: support Java type parameters
  userdiff: support Java record types
  userdiff: support Java sealed classes

 t/t4018/java-class-type-parameters                     | 6 ++++++
 t/t4018/java-class-type-parameters-implements          | 6 ++++++
 t/t4018/java-interface-type-parameters                 | 6 ++++++
 t/t4018/java-interface-type-parameters-extends         | 6 ++++++
 t/t4018/java-non-sealed                                | 8 ++++++++
 t/t4018/java-record                                    | 6 ++++++
 t/t4018/java-record-type-parameters                    | 6 ++++++
 t/t4018/java-sealed                                    | 7 +++++++
 t/t4018/java-sealed-permits                            | 6 ++++++
 t/t4018/java-sealed-type-parameters                    | 6 ++++++
 t/t4018/java-sealed-type-parameters-implements-permits | 6 ++++++
 t/t4018/java-sealed-type-parameters-permits            | 6 ++++++
 userdiff.c                                             | 2 +-
 13 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/java-class-type-parameters
 create mode 100644 t/t4018/java-class-type-parameters-implements
 create mode 100644 t/t4018/java-interface-type-parameters
 create mode 100644 t/t4018/java-interface-type-parameters-extends
 create mode 100644 t/t4018/java-non-sealed
 create mode 100644 t/t4018/java-record
 create mode 100644 t/t4018/java-record-type-parameters
 create mode 100644 t/t4018/java-sealed
 create mode 100644 t/t4018/java-sealed-permits
 create mode 100644 t/t4018/java-sealed-type-parameters
 create mode 100644 t/t4018/java-sealed-type-parameters-implements-permits
 create mode 100644 t/t4018/java-sealed-type-parameters-permits

-- 
2.39.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v1 1/3] userdiff: support Java type parameters
  2023-02-03 23:23 [PATCH v1 0/3] userdiff: Java updates Andrei Rybak
@ 2023-02-03 23:23 ` Andrei Rybak
  2023-02-03 23:23 ` [PATCH v1 2/3] userdiff: support Java record types Andrei Rybak
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-03 23:23 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Paolo Bonzini, Johannes Sixt, Tassilo Horn

A class or interface in Java [1] can have type parameters immediately
following the name in the declaration, surrounded by angle brackets
(paired less than and greater than signs).[2]  Example of a class with
type parameters "A" and "N":

    public class ParameterizedClass<A, N> {
        private A field1;
        private N field2;
    }

Support matching a parameterized class or interface declaration with
type parameters immediately following the name of the type in the
builtin userdiff pattern for Java.  Do so by just allowing matching the
first character after the name of the type to "<".

An alternative approach could be to match both the opening and the
closing angle brackets and matching the content between them in various
ways.  Just use the simpler regex for now.

[1] Since Java 5 released in 2004.
[2] Detailed description is available in the Java Language
    Specification, sections "Type Variables" and "Parameterized Types":
    https://docs.oracle.com/javase/specs/jls/se17/html/jls-4.html#jls-4.4

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t4018/java-class-type-parameters             | 6 ++++++
 t/t4018/java-class-type-parameters-implements  | 6 ++++++
 t/t4018/java-interface-type-parameters         | 6 ++++++
 t/t4018/java-interface-type-parameters-extends | 6 ++++++
 userdiff.c                                     | 2 +-
 5 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/java-class-type-parameters
 create mode 100644 t/t4018/java-class-type-parameters-implements
 create mode 100644 t/t4018/java-interface-type-parameters
 create mode 100644 t/t4018/java-interface-type-parameters-extends

diff --git a/t/t4018/java-class-type-parameters b/t/t4018/java-class-type-parameters
new file mode 100644
index 0000000000..579aa7af21
--- /dev/null
+++ b/t/t4018/java-class-type-parameters
@@ -0,0 +1,6 @@
+class RIGHT<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private A ChangeMe;
+}
diff --git a/t/t4018/java-class-type-parameters-implements b/t/t4018/java-class-type-parameters-implements
new file mode 100644
index 0000000000..b8038b1866
--- /dev/null
+++ b/t/t4018/java-class-type-parameters-implements
@@ -0,0 +1,6 @@
+class RIGHT<A, B> implements List<A> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private A ChangeMe;
+}
diff --git a/t/t4018/java-interface-type-parameters b/t/t4018/java-interface-type-parameters
new file mode 100644
index 0000000000..a4baa1ae68
--- /dev/null
+++ b/t/t4018/java-interface-type-parameters
@@ -0,0 +1,6 @@
+interface RIGHT<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    public B foo(A ChangeMe);
+}
diff --git a/t/t4018/java-interface-type-parameters-extends b/t/t4018/java-interface-type-parameters-extends
new file mode 100644
index 0000000000..31d7fb3244
--- /dev/null
+++ b/t/t4018/java-interface-type-parameters-extends
@@ -0,0 +1,6 @@
+interface RIGHT<A, B> extends Function<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    public B foo(A ChangeMe);
+}
diff --git a/userdiff.c b/userdiff.c
index d71b82feb7..759e22ffff 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -171,7 +171,7 @@ PATTERNS("html",
 PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
 	 /* Class, enum, and interface declarations */
-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*[ \t]+.*)$\n"
+	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|<).*)$\n"
 	 /* Method definitions; note that constructor signatures are not */
 	 /* matched because they are indistinguishable from method calls. */
 	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v1 2/3] userdiff: support Java record types
  2023-02-03 23:23 [PATCH v1 0/3] userdiff: Java updates Andrei Rybak
  2023-02-03 23:23 ` [PATCH v1 1/3] userdiff: support Java type parameters Andrei Rybak
@ 2023-02-03 23:23 ` Andrei Rybak
  2023-02-03 23:23 ` [PATCH v1 3/3] userdiff: support Java sealed classes Andrei Rybak
  2023-02-04  9:22 ` [PATCH v1 0/3] userdiff: Java updates Tassilo Horn
  3 siblings, 0 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-03 23:23 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Paolo Bonzini, Johannes Sixt, Tassilo Horn

A new kind of class was added in Java 16 -- records.[1]  The syntax of
records is similar to regular classes with one important distinction:
the name of the record class is followed by a mandatory list of
components.  The list is enclosed in parentheses, it may be empty, and
it may immediately follow the name of the class or type parameters, if
any, without separating whitespace.

Code examples:

    public record Example(int i, String s) {
    }

    public record WithTypeParameters<A, B>(A a, B b, String s) {
    }

Support records in the builtin userdiff pattern for Java.  Add "record"
to the alternatives of keywords for kinds of class, and match an opening
parenthesis as the first character right after the type name.

An alternative approach could be to have an optional group that would
match both the opening and the closing parentheses with some way of
matching the declarations of the components.  Just use the simpler
regular expression for now.

[1] detailed description is available in "JEP 395: Records"
    https://openjdk.org/jeps/395

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t4018/java-record                 | 6 ++++++
 t/t4018/java-record-type-parameters | 6 ++++++
 userdiff.c                          | 2 +-
 3 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/java-record
 create mode 100644 t/t4018/java-record-type-parameters

diff --git a/t/t4018/java-record b/t/t4018/java-record
new file mode 100644
index 0000000000..97aa819dd8
--- /dev/null
+++ b/t/t4018/java-record
@@ -0,0 +1,6 @@
+public record RIGHT(int comp1, double comp2, String comp3) {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    static int ChangeMe;
+}
diff --git a/t/t4018/java-record-type-parameters b/t/t4018/java-record-type-parameters
new file mode 100644
index 0000000000..f62a035cc8
--- /dev/null
+++ b/t/t4018/java-record-type-parameters
@@ -0,0 +1,6 @@
+public record RIGHT<A, N extends Number>(A comp1, N comp2, int comp3) {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    static int ChangeMe;
+}
diff --git a/userdiff.c b/userdiff.c
index 759e22ffff..f92b3029aa 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -171,7 +171,7 @@ PATTERNS("html",
 PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
 	 /* Class, enum, and interface declarations */
-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|<).*)$\n"
+	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
 	 /* Method definitions; note that constructor signatures are not */
 	 /* matched because they are indistinguishable from method calls. */
 	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v1 3/3] userdiff: support Java sealed classes
  2023-02-03 23:23 [PATCH v1 0/3] userdiff: Java updates Andrei Rybak
  2023-02-03 23:23 ` [PATCH v1 1/3] userdiff: support Java type parameters Andrei Rybak
  2023-02-03 23:23 ` [PATCH v1 2/3] userdiff: support Java record types Andrei Rybak
@ 2023-02-03 23:23 ` Andrei Rybak
  2023-02-04  9:22 ` [PATCH v1 0/3] userdiff: Java updates Tassilo Horn
  3 siblings, 0 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-03 23:23 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Paolo Bonzini, Johannes Sixt, Tassilo Horn

A new kind of class was added in Java 17 -- sealed classes.[1]  This
feature includes several new keywords that may appear in a declaration
of a class.  New modifiers before name of the class: "sealed" and
"non-sealed", and a clause after name of the class marked by keyword
"permits".

The current set of regular expressions in userdiff.c already allows the
modifier "sealed" and the "permits" clause, but not the modifier
"non-sealed", which is the first hyphenated keyword in Java.[2]  Allow
hyphen in the words that precede the name of type to match the
"non-sealed" modifier.

In new input file "java-sealed" for the test t4018-diff-funcname.sh, use
a Java code comment for the marker "RIGHT".  This workaround is needed,
because the name of the sealed class appears on the line of code that
has the "ChangeMe" marker.

[1] Detailed description in "JEP 409: Sealed Classes"
    https://openjdk.org/jeps/409
[2] "JEP draft: Keyword Management for the Java Language"
    https://openjdk.org/jeps/8223002

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t4018/java-non-sealed                                | 8 ++++++++
 t/t4018/java-sealed                                    | 7 +++++++
 t/t4018/java-sealed-permits                            | 6 ++++++
 t/t4018/java-sealed-type-parameters                    | 6 ++++++
 t/t4018/java-sealed-type-parameters-implements-permits | 6 ++++++
 t/t4018/java-sealed-type-parameters-permits            | 6 ++++++
 userdiff.c                                             | 2 +-
 7 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/java-non-sealed
 create mode 100644 t/t4018/java-sealed
 create mode 100644 t/t4018/java-sealed-permits
 create mode 100644 t/t4018/java-sealed-type-parameters
 create mode 100644 t/t4018/java-sealed-type-parameters-implements-permits
 create mode 100644 t/t4018/java-sealed-type-parameters-permits

diff --git a/t/t4018/java-non-sealed b/t/t4018/java-non-sealed
new file mode 100644
index 0000000000..f68ffd4ff3
--- /dev/null
+++ b/t/t4018/java-non-sealed
@@ -0,0 +1,8 @@
+public sealed abstract class SealedClass {
+    public static non-sealed class RIGHT extends SealedClass {
+        static int ONE;
+        static int TWO;
+        static int THREE;
+        private int ChangeMe;
+    }
+}
diff --git a/t/t4018/java-sealed b/t/t4018/java-sealed
new file mode 100644
index 0000000000..e722fee803
--- /dev/null
+++ b/t/t4018/java-sealed
@@ -0,0 +1,7 @@
+public sealed abstract class Sealed { // RIGHT
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    public final class ChangeMe extends Sealed {
+    }
+}
diff --git a/t/t4018/java-sealed-permits b/t/t4018/java-sealed-permits
new file mode 100644
index 0000000000..8573f2a7e8
--- /dev/null
+++ b/t/t4018/java-sealed-permits
@@ -0,0 +1,6 @@
+public sealed abstract class RIGHT permits PermittedA, PermittedB {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/t/t4018/java-sealed-type-parameters b/t/t4018/java-sealed-type-parameters
new file mode 100644
index 0000000000..ec31115961
--- /dev/null
+++ b/t/t4018/java-sealed-type-parameters
@@ -0,0 +1,6 @@
+public sealed abstract class RIGHT<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/t/t4018/java-sealed-type-parameters-implements-permits b/t/t4018/java-sealed-type-parameters-implements-permits
new file mode 100644
index 0000000000..9fd4dd5633
--- /dev/null
+++ b/t/t4018/java-sealed-type-parameters-implements-permits
@@ -0,0 +1,6 @@
+public sealed abstract class RIGHT<A, B> implements List<A> permits PermittedA, PermittedB {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/t/t4018/java-sealed-type-parameters-permits b/t/t4018/java-sealed-type-parameters-permits
new file mode 100644
index 0000000000..6af2352e46
--- /dev/null
+++ b/t/t4018/java-sealed-type-parameters-permits
@@ -0,0 +1,6 @@
+public sealed abstract class RIGHT<A, B> permits PermittedA, PermittedB {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/userdiff.c b/userdiff.c
index f92b3029aa..040deb7439 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -171,7 +171,7 @@ PATTERNS("html",
 PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
 	 /* Class, enum, and interface declarations */
-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
+	 "^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
 	 /* Method definitions; note that constructor signatures are not */
 	 /* matched because they are indistinguishable from method calls. */
 	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 0/3] userdiff: Java updates
  2023-02-03 23:23 [PATCH v1 0/3] userdiff: Java updates Andrei Rybak
                   ` (2 preceding siblings ...)
  2023-02-03 23:23 ` [PATCH v1 3/3] userdiff: support Java sealed classes Andrei Rybak
@ 2023-02-04  9:22 ` Tassilo Horn
  2023-02-04 13:43   ` [PATCH v2 " Andrei Rybak
  3 siblings, 1 reply; 19+ messages in thread
From: Tassilo Horn @ 2023-02-04  9:22 UTC (permalink / raw)
  To: Andrei Rybak; +Cc: git, Jeff King, Paolo Bonzini, Johannes Sixt

Andrei Rybak <rybak.a.v@gmail.com> writes:

Hi Andrei,

> Three patches to improve builtin userdiff support for Java features.
> Some pretty old features -- type parameters aka generics are actually
> older than Git, and some quite recent features of Java: records and
> sealed classes, released in Java 16 and Java 17 correspondingly, both
> from 2021.

Thanks for including me being the last contributor to java userdiff.
The patches look good from my POV and are safe-guarded with tests, so
I'm all for it.

Bye,
Tassilo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 0/3] userdiff: Java updates
  2023-02-04  9:22 ` [PATCH v1 0/3] userdiff: Java updates Tassilo Horn
@ 2023-02-04 13:43   ` Andrei Rybak
  2023-02-04 13:43     ` [PATCH v2 1/3] userdiff: support Java type parameters Andrei Rybak
                       ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-04 13:43 UTC (permalink / raw)
  To: Tassilo Horn, git; +Cc: Jeff King, Paolo Bonzini, Johannes Sixt

On 04/02/2023 10:22, Tassilo Horn wrote:
> Thanks for including me being the last contributor to java userdiff.
> The patches look good from my POV and are safe-guarded with tests, so
> I'm all for it.

Thank you for review!

I've realized that I've been writing modifiers "abstract" and "sealed" in a
technically correct, but not the conventional order.  Here's a reroll with the
order of modifiers following the style of original authors of
https://openjdk.org/jeps/409.  It doesn't matter for the purposes of the test,
but it will be less annoying to any future readers :-)

Range diff since v1:

1:  c300745a58 = 1:  c300745a58 userdiff: support Java type parameters
2:  a0e622a0f8 = 2:  a0e622a0f8 userdiff: support Java record types
3:  a53fca4d49 ! 3:  b9c6a5dffd userdiff: support Java sealed classes
    @@ Commit message
     
      ## t/t4018/java-non-sealed (new) ##
     @@
    -+public sealed abstract class SealedClass {
    ++public abstract sealed class SealedClass {
     +    public static non-sealed class RIGHT extends SealedClass {
     +        static int ONE;
     +        static int TWO;
    @@ t/t4018/java-non-sealed (new)
     
      ## t/t4018/java-sealed (new) ##
     @@
    -+public sealed abstract class Sealed { // RIGHT
    ++public abstract sealed class Sealed { // RIGHT
     +    static int ONE;
     +    static int TWO;
     +    static int THREE;
    @@ t/t4018/java-sealed (new)
     
      ## t/t4018/java-sealed-permits (new) ##
     @@
    -+public sealed abstract class RIGHT permits PermittedA, PermittedB {
    ++public abstract sealed class RIGHT permits PermittedA, PermittedB {
     +    static int ONE;
     +    static int TWO;
     +    static int THREE;
    @@ t/t4018/java-sealed-permits (new)
     
      ## t/t4018/java-sealed-type-parameters (new) ##
     @@
    -+public sealed abstract class RIGHT<A, B> {
    ++public abstract sealed class RIGHT<A, B> {
     +    static int ONE;
     +    static int TWO;
     +    static int THREE;
    @@ t/t4018/java-sealed-type-parameters (new)
     
      ## t/t4018/java-sealed-type-parameters-implements-permits (new) ##
     @@
    -+public sealed abstract class RIGHT<A, B> implements List<A> permits PermittedA, PermittedB {
    ++public abstract sealed class RIGHT<A, B> implements List<A> permits PermittedA, PermittedB {
     +    static int ONE;
     +    static int TWO;
     +    static int THREE;
    @@ t/t4018/java-sealed-type-parameters-implements-permits (new)
     
      ## t/t4018/java-sealed-type-parameters-permits (new) ##
     @@
    -+public sealed abstract class RIGHT<A, B> permits PermittedA, PermittedB {
    ++public abstract sealed class RIGHT<A, B> permits PermittedA, PermittedB {
     +    static int ONE;
     +    static int TWO;
     +    static int THREE;


Andrei Rybak (3):
  userdiff: support Java type parameters
  userdiff: support Java record types
  userdiff: support Java sealed classes

 t/t4018/java-class-type-parameters                     | 6 ++++++
 t/t4018/java-class-type-parameters-implements          | 6 ++++++
 t/t4018/java-interface-type-parameters                 | 6 ++++++
 t/t4018/java-interface-type-parameters-extends         | 6 ++++++
 t/t4018/java-non-sealed                                | 8 ++++++++
 t/t4018/java-record                                    | 6 ++++++
 t/t4018/java-record-type-parameters                    | 6 ++++++
 t/t4018/java-sealed                                    | 7 +++++++
 t/t4018/java-sealed-permits                            | 6 ++++++
 t/t4018/java-sealed-type-parameters                    | 6 ++++++
 t/t4018/java-sealed-type-parameters-implements-permits | 6 ++++++
 t/t4018/java-sealed-type-parameters-permits            | 6 ++++++
 userdiff.c                                             | 2 +-
 13 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/java-class-type-parameters
 create mode 100644 t/t4018/java-class-type-parameters-implements
 create mode 100644 t/t4018/java-interface-type-parameters
 create mode 100644 t/t4018/java-interface-type-parameters-extends
 create mode 100644 t/t4018/java-non-sealed
 create mode 100644 t/t4018/java-record
 create mode 100644 t/t4018/java-record-type-parameters
 create mode 100644 t/t4018/java-sealed
 create mode 100644 t/t4018/java-sealed-permits
 create mode 100644 t/t4018/java-sealed-type-parameters
 create mode 100644 t/t4018/java-sealed-type-parameters-implements-permits
 create mode 100644 t/t4018/java-sealed-type-parameters-permits

-- 
2.39.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/3] userdiff: support Java type parameters
  2023-02-04 13:43   ` [PATCH v2 " Andrei Rybak
@ 2023-02-04 13:43     ` Andrei Rybak
  2023-02-04 13:43     ` [PATCH v2 2/3] userdiff: support Java record types Andrei Rybak
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-04 13:43 UTC (permalink / raw)
  To: Tassilo Horn, git; +Cc: Jeff King, Paolo Bonzini, Johannes Sixt

A class or interface in Java [1] can have type parameters immediately
following the name in the declaration, surrounded by angle brackets
(paired less than and greater than signs).[2]  Example of a class with
type parameters "A" and "N":

    public class ParameterizedClass<A, N> {
        private A field1;
        private N field2;
    }

Support matching a parameterized class or interface declaration with
type parameters immediately following the name of the type in the
builtin userdiff pattern for Java.  Do so by just allowing matching the
first character after the name of the type to "<".

An alternative approach could be to match both the opening and the
closing angle brackets and matching the content between them in various
ways.  Just use the simpler regex for now.

[1] Since Java 5 released in 2004.
[2] Detailed description is available in the Java Language
    Specification, sections "Type Variables" and "Parameterized Types":
    https://docs.oracle.com/javase/specs/jls/se17/html/jls-4.html#jls-4.4

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t4018/java-class-type-parameters             | 6 ++++++
 t/t4018/java-class-type-parameters-implements  | 6 ++++++
 t/t4018/java-interface-type-parameters         | 6 ++++++
 t/t4018/java-interface-type-parameters-extends | 6 ++++++
 userdiff.c                                     | 2 +-
 5 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/java-class-type-parameters
 create mode 100644 t/t4018/java-class-type-parameters-implements
 create mode 100644 t/t4018/java-interface-type-parameters
 create mode 100644 t/t4018/java-interface-type-parameters-extends

diff --git a/t/t4018/java-class-type-parameters b/t/t4018/java-class-type-parameters
new file mode 100644
index 0000000000..579aa7af21
--- /dev/null
+++ b/t/t4018/java-class-type-parameters
@@ -0,0 +1,6 @@
+class RIGHT<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private A ChangeMe;
+}
diff --git a/t/t4018/java-class-type-parameters-implements b/t/t4018/java-class-type-parameters-implements
new file mode 100644
index 0000000000..b8038b1866
--- /dev/null
+++ b/t/t4018/java-class-type-parameters-implements
@@ -0,0 +1,6 @@
+class RIGHT<A, B> implements List<A> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private A ChangeMe;
+}
diff --git a/t/t4018/java-interface-type-parameters b/t/t4018/java-interface-type-parameters
new file mode 100644
index 0000000000..a4baa1ae68
--- /dev/null
+++ b/t/t4018/java-interface-type-parameters
@@ -0,0 +1,6 @@
+interface RIGHT<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    public B foo(A ChangeMe);
+}
diff --git a/t/t4018/java-interface-type-parameters-extends b/t/t4018/java-interface-type-parameters-extends
new file mode 100644
index 0000000000..31d7fb3244
--- /dev/null
+++ b/t/t4018/java-interface-type-parameters-extends
@@ -0,0 +1,6 @@
+interface RIGHT<A, B> extends Function<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    public B foo(A ChangeMe);
+}
diff --git a/userdiff.c b/userdiff.c
index d71b82feb7..759e22ffff 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -171,7 +171,7 @@ PATTERNS("html",
 PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
 	 /* Class, enum, and interface declarations */
-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*[ \t]+.*)$\n"
+	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|<).*)$\n"
 	 /* Method definitions; note that constructor signatures are not */
 	 /* matched because they are indistinguishable from method calls. */
 	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/3] userdiff: support Java record types
  2023-02-04 13:43   ` [PATCH v2 " Andrei Rybak
  2023-02-04 13:43     ` [PATCH v2 1/3] userdiff: support Java type parameters Andrei Rybak
@ 2023-02-04 13:43     ` Andrei Rybak
  2023-02-04 13:43     ` [PATCH v2 3/3] userdiff: support Java sealed classes Andrei Rybak
  2023-02-05 10:09     ` [PATCH v2 0/3] userdiff: Java updates Johannes Sixt
  3 siblings, 0 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-04 13:43 UTC (permalink / raw)
  To: Tassilo Horn, git; +Cc: Jeff King, Paolo Bonzini, Johannes Sixt

A new kind of class was added in Java 16 -- records.[1]  The syntax of
records is similar to regular classes with one important distinction:
the name of the record class is followed by a mandatory list of
components.  The list is enclosed in parentheses, it may be empty, and
it may immediately follow the name of the class or type parameters, if
any, without separating whitespace.

Code examples:

    public record Example(int i, String s) {
    }

    public record WithTypeParameters<A, B>(A a, B b, String s) {
    }

Support records in the builtin userdiff pattern for Java.  Add "record"
to the alternatives of keywords for kinds of class, and match an opening
parenthesis as the first character right after the type name.

An alternative approach could be to have an optional group that would
match both the opening and the closing parentheses with some way of
matching the declarations of the components.  Just use the simpler
regular expression for now.

[1] detailed description is available in "JEP 395: Records"
    https://openjdk.org/jeps/395

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t4018/java-record                 | 6 ++++++
 t/t4018/java-record-type-parameters | 6 ++++++
 userdiff.c                          | 2 +-
 3 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/java-record
 create mode 100644 t/t4018/java-record-type-parameters

diff --git a/t/t4018/java-record b/t/t4018/java-record
new file mode 100644
index 0000000000..97aa819dd8
--- /dev/null
+++ b/t/t4018/java-record
@@ -0,0 +1,6 @@
+public record RIGHT(int comp1, double comp2, String comp3) {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    static int ChangeMe;
+}
diff --git a/t/t4018/java-record-type-parameters b/t/t4018/java-record-type-parameters
new file mode 100644
index 0000000000..f62a035cc8
--- /dev/null
+++ b/t/t4018/java-record-type-parameters
@@ -0,0 +1,6 @@
+public record RIGHT<A, N extends Number>(A comp1, N comp2, int comp3) {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    static int ChangeMe;
+}
diff --git a/userdiff.c b/userdiff.c
index 759e22ffff..f92b3029aa 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -171,7 +171,7 @@ PATTERNS("html",
 PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
 	 /* Class, enum, and interface declarations */
-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|<).*)$\n"
+	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
 	 /* Method definitions; note that constructor signatures are not */
 	 /* matched because they are indistinguishable from method calls. */
 	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 3/3] userdiff: support Java sealed classes
  2023-02-04 13:43   ` [PATCH v2 " Andrei Rybak
  2023-02-04 13:43     ` [PATCH v2 1/3] userdiff: support Java type parameters Andrei Rybak
  2023-02-04 13:43     ` [PATCH v2 2/3] userdiff: support Java record types Andrei Rybak
@ 2023-02-04 13:43     ` Andrei Rybak
  2023-02-05 10:09     ` [PATCH v2 0/3] userdiff: Java updates Johannes Sixt
  3 siblings, 0 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-04 13:43 UTC (permalink / raw)
  To: Tassilo Horn, git; +Cc: Jeff King, Paolo Bonzini, Johannes Sixt

A new kind of class was added in Java 17 -- sealed classes.[1]  This
feature includes several new keywords that may appear in a declaration
of a class.  New modifiers before name of the class: "sealed" and
"non-sealed", and a clause after name of the class marked by keyword
"permits".

The current set of regular expressions in userdiff.c already allows the
modifier "sealed" and the "permits" clause, but not the modifier
"non-sealed", which is the first hyphenated keyword in Java.[2]  Allow
hyphen in the words that precede the name of type to match the
"non-sealed" modifier.

In new input file "java-sealed" for the test t4018-diff-funcname.sh, use
a Java code comment for the marker "RIGHT".  This workaround is needed,
because the name of the sealed class appears on the line of code that
has the "ChangeMe" marker.

[1] Detailed description in "JEP 409: Sealed Classes"
    https://openjdk.org/jeps/409
[2] "JEP draft: Keyword Management for the Java Language"
    https://openjdk.org/jeps/8223002

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t4018/java-non-sealed                                | 8 ++++++++
 t/t4018/java-sealed                                    | 7 +++++++
 t/t4018/java-sealed-permits                            | 6 ++++++
 t/t4018/java-sealed-type-parameters                    | 6 ++++++
 t/t4018/java-sealed-type-parameters-implements-permits | 6 ++++++
 t/t4018/java-sealed-type-parameters-permits            | 6 ++++++
 userdiff.c                                             | 2 +-
 7 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/java-non-sealed
 create mode 100644 t/t4018/java-sealed
 create mode 100644 t/t4018/java-sealed-permits
 create mode 100644 t/t4018/java-sealed-type-parameters
 create mode 100644 t/t4018/java-sealed-type-parameters-implements-permits
 create mode 100644 t/t4018/java-sealed-type-parameters-permits

diff --git a/t/t4018/java-non-sealed b/t/t4018/java-non-sealed
new file mode 100644
index 0000000000..069087c1c6
--- /dev/null
+++ b/t/t4018/java-non-sealed
@@ -0,0 +1,8 @@
+public abstract sealed class SealedClass {
+    public static non-sealed class RIGHT extends SealedClass {
+        static int ONE;
+        static int TWO;
+        static int THREE;
+        private int ChangeMe;
+    }
+}
diff --git a/t/t4018/java-sealed b/t/t4018/java-sealed
new file mode 100644
index 0000000000..785fbc62bc
--- /dev/null
+++ b/t/t4018/java-sealed
@@ -0,0 +1,7 @@
+public abstract sealed class Sealed { // RIGHT
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    public final class ChangeMe extends Sealed {
+    }
+}
diff --git a/t/t4018/java-sealed-permits b/t/t4018/java-sealed-permits
new file mode 100644
index 0000000000..18dd4894cf
--- /dev/null
+++ b/t/t4018/java-sealed-permits
@@ -0,0 +1,6 @@
+public abstract sealed class RIGHT permits PermittedA, PermittedB {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/t/t4018/java-sealed-type-parameters b/t/t4018/java-sealed-type-parameters
new file mode 100644
index 0000000000..e6530c47c3
--- /dev/null
+++ b/t/t4018/java-sealed-type-parameters
@@ -0,0 +1,6 @@
+public abstract sealed class RIGHT<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/t/t4018/java-sealed-type-parameters-implements-permits b/t/t4018/java-sealed-type-parameters-implements-permits
new file mode 100644
index 0000000000..bd6e6d3582
--- /dev/null
+++ b/t/t4018/java-sealed-type-parameters-implements-permits
@@ -0,0 +1,6 @@
+public abstract sealed class RIGHT<A, B> implements List<A> permits PermittedA, PermittedB {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/t/t4018/java-sealed-type-parameters-permits b/t/t4018/java-sealed-type-parameters-permits
new file mode 100644
index 0000000000..25a0da6442
--- /dev/null
+++ b/t/t4018/java-sealed-type-parameters-permits
@@ -0,0 +1,6 @@
+public abstract sealed class RIGHT<A, B> permits PermittedA, PermittedB {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/userdiff.c b/userdiff.c
index f92b3029aa..040deb7439 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -171,7 +171,7 @@ PATTERNS("html",
 PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
 	 /* Class, enum, and interface declarations */
-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
+	 "^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
 	 /* Method definitions; note that constructor signatures are not */
 	 /* matched because they are indistinguishable from method calls. */
 	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/3] userdiff: Java updates
  2023-02-04 13:43   ` [PATCH v2 " Andrei Rybak
                       ` (2 preceding siblings ...)
  2023-02-04 13:43     ` [PATCH v2 3/3] userdiff: support Java sealed classes Andrei Rybak
@ 2023-02-05 10:09     ` Johannes Sixt
  2023-02-05 19:27       ` Andrei Rybak
  3 siblings, 1 reply; 19+ messages in thread
From: Johannes Sixt @ 2023-02-05 10:09 UTC (permalink / raw)
  To: Andrei Rybak; +Cc: Jeff King, Paolo Bonzini, git, Tassilo Horn

Am 04.02.23 um 14:43 schrieb Andrei Rybak:
> On 04/02/2023 10:22, Tassilo Horn wrote:
>> Thanks for including me being the last contributor to java userdiff.
>> The patches look good from my POV and are safe-guarded with tests, so
>> I'm all for it.
> 
> Thank you for review!
> 
> I've realized that I've been writing modifiers "abstract" and "sealed" in a
> technically correct, but not the conventional order.  Here's a reroll with the
> order of modifiers following the style of original authors of
> https://openjdk.org/jeps/409.  It doesn't matter for the purposes of the test,
> but it will be less annoying to any future readers :-)

I've looked through the patches and run the tests, and they all make
sense to me. By just looking at the patch text I noted that no
whitespace between the identifier and the opening angle bracket is
permitted and whether it should be allowed, but the commit messages make
quite clear that whitespace is not allowed in this position. Hence:

Reviewed-by: Johannes Sixt <j6t@kdbg.org>

Thanks,
-- Hannes


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/3] userdiff: Java updates
  2023-02-05 10:09     ` [PATCH v2 0/3] userdiff: Java updates Johannes Sixt
@ 2023-02-05 19:27       ` Andrei Rybak
  2023-02-05 21:33         ` Johannes Sixt
  0 siblings, 1 reply; 19+ messages in thread
From: Andrei Rybak @ 2023-02-05 19:27 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jeff King, Paolo Bonzini, git, Tassilo Horn

On 2023-02-05T11:09, Johannes Sixt wrote:
> Am 04.02.23 um 14:43 schrieb Andrei Rybak:
>> On 04/02/2023 10:22, Tassilo Horn wrote:
>>> Thanks for including me being the last contributor to java userdiff.
>>> The patches look good from my POV and are safe-guarded with tests, so
>>> I'm all for it.
>>
>> Thank you for review!
>>
>> I've realized that I've been writing modifiers "abstract" and "sealed" in a
>> technically correct, but not the conventional order.  Here's a reroll with the
>> order of modifiers following the style of original authors of
>> https://openjdk.org/jeps/409.  It doesn't matter for the purposes of the test,
>> but it will be less annoying to any future readers :-)
> 
> I've looked through the patches and run the tests, and they all make
> sense to me. By just looking at the patch text I noted that no
> whitespace between the identifier and the opening angle bracket is
> permitted and whether it should be allowed, but the commit messages make
> quite clear that whitespace is not allowed in this position.

There is some kind of misunderstanding.  I guess the wording in commit
messages of the first and second patches could have been clearer.

In Java, whitespace is allowed between type name and the brackets.
It is permitted both for angle brackets of type parameters:

	class SpacesBeforeTypeParameters         <A, B> {
	}

and for round brackets of components in records:

	record SpacesBeforeComponents      (String comp1, int comp2) {
	}

The common convention, is however, to omit the whitespace before the
brackets.

The regular expression on branch master already allows for whitespace
after the name of the type:

	"^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*[ \t]+.*)$\n"
	                                                                          ^^^^^^
so I didn't need to cover this case.  Note that it requires a non-zero
amount of whitespace. This part of the regular expression was left as
is (v2 after patch 3/3):

	"^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
	                                                                                   ^^^^^^


That being said, I guess it would be an improvement to also allow
the name of the type be followed by the end of the line, for users
with fairly common code style that puts braces on separate lines:

	class WithLineBreakBeforeOpeningBrace
	{
	}

or `extends` and `implements` clauses after a line break:

	class ExtendsOnSeparateLine
		extends Number
		implements Serializable
	{
	}

even type parameters:

	class TypeParametersOnSeparateLine
		<A, B>
	{
	}

Something like the following:

	"^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*(([ \t]+|[<(]).*)?)$\n"
	                                                                                  ^               ^^
perhaps? Technically, the following is also valid Java:

	class WithComment//comment immediately after class name
	{
	}

but I'm not sure if allowing it is needed.  If so, we might as well just do this:

	"^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*.*)$\n"
	                                                                                  ^^

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/3] userdiff: Java updates
  2023-02-05 19:27       ` Andrei Rybak
@ 2023-02-05 21:33         ` Johannes Sixt
  2023-02-07 23:42           ` [PATCH v3 " Andrei Rybak
  0 siblings, 1 reply; 19+ messages in thread
From: Johannes Sixt @ 2023-02-05 21:33 UTC (permalink / raw)
  To: Andrei Rybak; +Cc: Jeff King, Paolo Bonzini, git, Tassilo Horn

Am 05.02.23 um 20:27 schrieb Andrei Rybak:
> On 2023-02-05T11:09, Johannes Sixt wrote:
>> Am 04.02.23 um 14:43 schrieb Andrei Rybak:
>>> On 04/02/2023 10:22, Tassilo Horn wrote:
>>>> Thanks for including me being the last contributor to java userdiff.
>>>> The patches look good from my POV and are safe-guarded with tests, so
>>>> I'm all for it.
>>>
>>> Thank you for review!
>>>
>>> I've realized that I've been writing modifiers "abstract" and
>>> "sealed" in a
>>> technically correct, but not the conventional order.  Here's a reroll
>>> with the
>>> order of modifiers following the style of original authors of
>>> https://openjdk.org/jeps/409.  It doesn't matter for the purposes of
>>> the test,
>>> but it will be less annoying to any future readers :-)
>>
>> I've looked through the patches and run the tests, and they all make
>> sense to me. By just looking at the patch text I noted that no
>> whitespace between the identifier and the opening angle bracket is
>> permitted and whether it should be allowed, but the commit messages make
>> quite clear that whitespace is not allowed in this position.
> 
> There is some kind of misunderstanding.  I guess the wording in commit
> messages of the first and second patches could have been clearer.
> 
> In Java, whitespace is allowed between type name and the brackets.
> It is permitted both for angle brackets of type parameters:
> 
>     class SpacesBeforeTypeParameters         <A, B> {
>     }
> 
> and for round brackets of components in records:
> 
>     record SpacesBeforeComponents      (String comp1, int comp2) {
>     }
> 
> The common convention, is however, to omit the whitespace before the
> brackets.
> 
> The regular expression on branch master already allows for whitespace
> after the name of the type:
> 
>     "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[
> \t]+[A-Za-z][A-Za-z0-9_$]*[ \t]+.*)$\n"
>                                                                               ^^^^^^
> so I didn't need to cover this case.  Note that it requires a non-zero
> amount of whitespace. This part of the regular expression was left as
> is (v2 after patch 3/3):
> 
>     "^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[
> \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
>                                                                                        ^^^^^^
> 
> 
> That being said, I guess it would be an improvement to also allow
> the name of the type be followed by the end of the line, for users
> with fairly common code style that puts braces on separate lines:
> 
>     class WithLineBreakBeforeOpeningBrace
>     {
>     }
> 
> or `extends` and `implements` clauses after a line break:
> 
>     class ExtendsOnSeparateLine
>         extends Number
>         implements Serializable
>     {
>     }
> 
> even type parameters:
> 
>     class TypeParametersOnSeparateLine
>         <A, B>
>     {
>     }
> 
> Something like the following:
> 
>     "^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[
> \t]+[A-Za-z][A-Za-z0-9_$]*(([ \t]+|[<(]).*)?)$\n"
>                                                                                       ^               ^^
> perhaps? Technically, the following is also valid Java:
> 
>     class WithComment//comment immediately after class name
>     {
>     }
> 
> but I'm not sure if allowing it is needed.  If so, we might as well just
> do this:
> 
>     "^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[
> \t]+[A-Za-z][A-Za-z0-9_$]*.*)$\n"
>                                                                                       ^^

Having seen all these examples, I think the following truncated
expression might do the right thing for all cases that are valid Java:

"^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t].*)$"

i.e., we recognize a whitespace in order to identify the keyword, and
then capture anything that follows without being specific. My reasoning
is that "class", "enum", "interface", and "record" cannot occur in any
other context than the beginning of a class definition. (But please do
correct me; I know next to nothing about Java syntax.) As always,
userdiff regular expressions can assume that only valid constructs are
inspected.

-- Hannes


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 0/3] userdiff: Java updates
  2023-02-05 21:33         ` Johannes Sixt
@ 2023-02-07 23:42           ` Andrei Rybak
  2023-02-07 23:42             ` [PATCH v3 1/3] userdiff: support Java type parameters Andrei Rybak
                               ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-07 23:42 UTC (permalink / raw)
  To: Johannes Sixt, git; +Cc: Jeff King, Paolo Bonzini, Tassilo Horn

On 2023-02-05T22:33 Johannes Sixt wrote:
> Having seen all these examples, I think the following truncated
> expression might do the right thing for all cases that are valid Java:
> 
> "^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t].*)$"

Only the '\n' is missing at the end, but otherwise I concur, so here's a v3.

> i.e., we recognize a whitespace in order to identify the keyword, and
> then capture anything that follows without being specific. My reasoning
> is that "class", "enum", "interface", and "record" cannot occur in any
> other context than the beginning of a class definition. (But please do
> correct me; I know next to nothing about Java syntax.)

The word "class" can also occur as part of a class literal, for example:

    Class<String> c = String.class;

but valid uses of class literals won't interfere with our regex, unless some
wild formatting is applied.  This is technically valid Java:

    Class<String> c = String.
    class 
    ;

and with a space after lowercase "class", the v3 regex will trip.  Class
literals are described in the JLS here:
https://docs.oracle.com/javase/specs/jls/se17/html/jls-15.html#jls-15.8.2

> As always,
> userdiff regular expressions can assume that only valid constructs are
> inspected.

Changes since v2:

  - simplified regex that doesn't match class names at all and supports more
    code styles
  - updated the comment just above the regex in PATCH 2/3 to mention records
  - more tests to cover the cases mentioned during review of v2
  - reworded commit messages to reflect the above items

Range diff since v2:

1:  c300745a58 ! 1:  9e859e3b79 userdiff: support Java type parameters
    @@ Metadata
      ## Commit message ##
         userdiff: support Java type parameters
     
    -    A class or interface in Java [1] can have type parameters immediately
    -    following the name in the declaration, surrounded by angle brackets
    -    (paired less than and greater than signs).[2]  Example of a class with
    -    type parameters "A" and "N":
    -
    -        public class ParameterizedClass<A, N> {
    -            private A field1;
    -            private N field2;
    +    A class or interface in Java can have type parameters following the name
    +    in the declared type, surrounded by angle brackets (paired less than and
    +    greater than signs).[2]   The type parameters -- `A` and `B` in the
    +    examples -- may follow the class name immediately:
    +
    +        public class ParameterizedClass<A, B> {
             }
     
    -    Support matching a parameterized class or interface declaration with
    -    type parameters immediately following the name of the type in the
    -    builtin userdiff pattern for Java.  Do so by just allowing matching the
    -    first character after the name of the type to "<".
    +    or may be separated by whitespace:
    +
    +        public class SpaceBeforeTypeParameters <A, B> {
    +        }
     
    -    An alternative approach could be to match both the opening and the
    -    closing angle brackets and matching the content between them in various
    -    ways.  Just use the simpler regex for now.
    +    A part of the builtin userdiff pattern for Java matches declarations of
    +    classes, enums, and interfaces.  The regular expression requires at
    +    least one whitespace character after the name of the declared type.
    +    This disallows matching for opening angle bracket of type parameters
    +    immediately after the name of the type.  Mandatory whitespace after the
    +    name of the type also disallows using the pattern in repositories with a
    +    fairly common code style that puts braces for the body of a class on
    +    separate lines:
    +
    +        class WithLineBreakBeforeOpeningBrace
    +        {
    +        }
    +
    +    Support matching Java code in more diverse code styles and declarations
    +    of classes and interfaces with type parameters immediately following the
    +    name of the type in the builtin userdiff pattern for Java.  Do so by
    +    just matching anything until the end of the line after the keywords for
    +    the kind of type being declared.
     
         [1] Since Java 5 released in 2004.
         [2] Detailed description is available in the Java Language
    @@ Commit message
     
         Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
     
    + ## t/t4018/java-class-brace-on-separate-line (new) ##
    +@@
    ++class RIGHT
    ++{
    ++    static int ONE;
    ++    static int TWO;
    ++    static int ChangeMe;
    ++}
    +
    + ## t/t4018/java-class-space-before-type-parameters (new) ##
    +@@
    ++class RIGHT <TYPE, PARAMS, AFTER, SPACE> {
    ++    static int ONE;
    ++    static int TWO;
    ++    static int THREE;
    ++    private A ChangeMe;
    ++}
    +
      ## t/t4018/java-class-type-parameters (new) ##
     @@
     +class RIGHT<A, B> {
    @@ userdiff.c: PATTERNS("html",
      	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
      	 /* Class, enum, and interface declarations */
     -	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*[ \t]+.*)$\n"
    -+	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|<).*)$\n"
    ++	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+.*)$\n"
      	 /* Method definitions; note that constructor signatures are not */
      	 /* matched because they are indistinguishable from method calls. */
      	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
2:  a0e622a0f8 ! 2:  4f7be5f642 userdiff: support Java record types
    @@ Commit message
         the name of the record class is followed by a mandatory list of
         components.  The list is enclosed in parentheses, it may be empty, and
         it may immediately follow the name of the class or type parameters, if
    -    any, without separating whitespace.
    -
    -    Code examples:
    +    any, with or without separating whitespace.  For example:
     
             public record Example(int i, String s) {
             }
    @@ Commit message
             public record WithTypeParameters<A, B>(A a, B b, String s) {
             }
     
    +        record SpaceBeforeComponents (String comp1, int comp2) {
    +        }
    +
         Support records in the builtin userdiff pattern for Java.  Add "record"
    -    to the alternatives of keywords for kinds of class, and match an opening
    -    parenthesis as the first character right after the type name.
    +    to the alternatives of keywords for kinds of class.
     
    -    An alternative approach could be to have an optional group that would
    -    match both the opening and the closing parentheses with some way of
    -    matching the declarations of the components.  Just use the simpler
    -    regular expression for now.
    +    Allowing matching various possibilities for the type parameters and/or
    +    list of the components of a record has already been covered by the
    +    preceding patch.
     
         [1] detailed description is available in "JEP 395: Records"
             https://openjdk.org/jeps/395
    @@ t/t4018/java-record (new)
     +    static int TWO;
     +    static int THREE;
     +    static int ChangeMe;
    ++}
    +
    + ## t/t4018/java-record-space-before-components (new) ##
    +@@
    ++public record RIGHT (String components, String after, String space) {
    ++    static int ONE;
    ++    static int TWO;
    ++    static int THREE;
    ++    static int ChangeMe;
     +}
     
      ## t/t4018/java-record-type-parameters (new) ##
    @@ t/t4018/java-record-type-parameters (new)
     
      ## userdiff.c ##
     @@ userdiff.c: PATTERNS("html",
    + 	 "[^<>= \t]+"),
      PATTERNS("java",
      	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
    - 	 /* Class, enum, and interface declarations */
    --	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|<).*)$\n"
    -+	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
    +-	 /* Class, enum, and interface declarations */
    +-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+.*)$\n"
    ++	 /* Class, enum, interface, and record declarations */
    ++	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface|record)[ \t]+.*)$\n"
      	 /* Method definitions; note that constructor signatures are not */
      	 /* matched because they are indistinguishable from method calls. */
      	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
3:  b9c6a5dffd ! 3:  ea6ce671ef userdiff: support Java sealed classes
    @@ userdiff.c
     @@ userdiff.c: PATTERNS("html",
      PATTERNS("java",
      	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
    - 	 /* Class, enum, and interface declarations */
    --	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
    -+	 "^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t]+[A-Za-z][A-Za-z0-9_$]*([ \t]+|[<(]).*)$\n"
    + 	 /* Class, enum, interface, and record declarations */
    +-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface|record)[ \t]+.*)$\n"
    ++	 "^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t]+.*)$\n"
      	 /* Method definitions; note that constructor signatures are not */
      	 /* matched because they are indistinguishable from method calls. */
      	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",

Andrei Rybak (3):
  userdiff: support Java type parameters
  userdiff: support Java record types
  userdiff: support Java sealed classes

 t/t4018/java-class-brace-on-separate-line              | 6 ++++++
 t/t4018/java-class-space-before-type-parameters        | 6 ++++++
 t/t4018/java-class-type-parameters                     | 6 ++++++
 t/t4018/java-class-type-parameters-implements          | 6 ++++++
 t/t4018/java-interface-type-parameters                 | 6 ++++++
 t/t4018/java-interface-type-parameters-extends         | 6 ++++++
 t/t4018/java-non-sealed                                | 8 ++++++++
 t/t4018/java-record                                    | 6 ++++++
 t/t4018/java-record-space-before-components            | 6 ++++++
 t/t4018/java-record-type-parameters                    | 6 ++++++
 t/t4018/java-sealed                                    | 7 +++++++
 t/t4018/java-sealed-permits                            | 6 ++++++
 t/t4018/java-sealed-type-parameters                    | 6 ++++++
 t/t4018/java-sealed-type-parameters-implements-permits | 6 ++++++
 t/t4018/java-sealed-type-parameters-permits            | 6 ++++++
 userdiff.c                                             | 4 ++--
 16 files changed, 95 insertions(+), 2 deletions(-)
 create mode 100644 t/t4018/java-class-brace-on-separate-line
 create mode 100644 t/t4018/java-class-space-before-type-parameters
 create mode 100644 t/t4018/java-class-type-parameters
 create mode 100644 t/t4018/java-class-type-parameters-implements
 create mode 100644 t/t4018/java-interface-type-parameters
 create mode 100644 t/t4018/java-interface-type-parameters-extends
 create mode 100644 t/t4018/java-non-sealed
 create mode 100644 t/t4018/java-record
 create mode 100644 t/t4018/java-record-space-before-components
 create mode 100644 t/t4018/java-record-type-parameters
 create mode 100644 t/t4018/java-sealed
 create mode 100644 t/t4018/java-sealed-permits
 create mode 100644 t/t4018/java-sealed-type-parameters
 create mode 100644 t/t4018/java-sealed-type-parameters-implements-permits
 create mode 100644 t/t4018/java-sealed-type-parameters-permits

-- 
2.39.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 1/3] userdiff: support Java type parameters
  2023-02-07 23:42           ` [PATCH v3 " Andrei Rybak
@ 2023-02-07 23:42             ` Andrei Rybak
  2023-02-08  0:04               ` Andrei Rybak
  2023-02-07 23:42             ` [PATCH v3 2/3] userdiff: support Java record types Andrei Rybak
                               ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Andrei Rybak @ 2023-02-07 23:42 UTC (permalink / raw)
  To: Johannes Sixt, git; +Cc: Jeff King, Paolo Bonzini, Tassilo Horn

A class or interface in Java can have type parameters following the name
in the declared type, surrounded by angle brackets (paired less than and
greater than signs).[2]   The type parameters -- `A` and `B` in the
examples -- may follow the class name immediately:

    public class ParameterizedClass<A, B> {
    }

or may be separated by whitespace:

    public class SpaceBeforeTypeParameters <A, B> {
    }

A part of the builtin userdiff pattern for Java matches declarations of
classes, enums, and interfaces.  The regular expression requires at
least one whitespace character after the name of the declared type.
This disallows matching for opening angle bracket of type parameters
immediately after the name of the type.  Mandatory whitespace after the
name of the type also disallows using the pattern in repositories with a
fairly common code style that puts braces for the body of a class on
separate lines:

    class WithLineBreakBeforeOpeningBrace
    {
    }

Support matching Java code in more diverse code styles and declarations
of classes and interfaces with type parameters immediately following the
name of the type in the builtin userdiff pattern for Java.  Do so by
just matching anything until the end of the line after the keywords for
the kind of type being declared.

[1] Since Java 5 released in 2004.
[2] Detailed description is available in the Java Language
    Specification, sections "Type Variables" and "Parameterized Types":
    https://docs.oracle.com/javase/specs/jls/se17/html/jls-4.html#jls-4.4

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t4018/java-class-brace-on-separate-line       | 6 ++++++
 t/t4018/java-class-space-before-type-parameters | 6 ++++++
 t/t4018/java-class-type-parameters              | 6 ++++++
 t/t4018/java-class-type-parameters-implements   | 6 ++++++
 t/t4018/java-interface-type-parameters          | 6 ++++++
 t/t4018/java-interface-type-parameters-extends  | 6 ++++++
 userdiff.c                                      | 2 +-
 7 files changed, 37 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/java-class-brace-on-separate-line
 create mode 100644 t/t4018/java-class-space-before-type-parameters
 create mode 100644 t/t4018/java-class-type-parameters
 create mode 100644 t/t4018/java-class-type-parameters-implements
 create mode 100644 t/t4018/java-interface-type-parameters
 create mode 100644 t/t4018/java-interface-type-parameters-extends

diff --git a/t/t4018/java-class-brace-on-separate-line b/t/t4018/java-class-brace-on-separate-line
new file mode 100644
index 0000000000..8795acd4cf
--- /dev/null
+++ b/t/t4018/java-class-brace-on-separate-line
@@ -0,0 +1,6 @@
+class RIGHT
+{
+    static int ONE;
+    static int TWO;
+    static int ChangeMe;
+}
diff --git a/t/t4018/java-class-space-before-type-parameters b/t/t4018/java-class-space-before-type-parameters
new file mode 100644
index 0000000000..0bdef1dfbe
--- /dev/null
+++ b/t/t4018/java-class-space-before-type-parameters
@@ -0,0 +1,6 @@
+class RIGHT <TYPE, PARAMS, AFTER, SPACE> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private A ChangeMe;
+}
diff --git a/t/t4018/java-class-type-parameters b/t/t4018/java-class-type-parameters
new file mode 100644
index 0000000000..579aa7af21
--- /dev/null
+++ b/t/t4018/java-class-type-parameters
@@ -0,0 +1,6 @@
+class RIGHT<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private A ChangeMe;
+}
diff --git a/t/t4018/java-class-type-parameters-implements b/t/t4018/java-class-type-parameters-implements
new file mode 100644
index 0000000000..b8038b1866
--- /dev/null
+++ b/t/t4018/java-class-type-parameters-implements
@@ -0,0 +1,6 @@
+class RIGHT<A, B> implements List<A> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private A ChangeMe;
+}
diff --git a/t/t4018/java-interface-type-parameters b/t/t4018/java-interface-type-parameters
new file mode 100644
index 0000000000..a4baa1ae68
--- /dev/null
+++ b/t/t4018/java-interface-type-parameters
@@ -0,0 +1,6 @@
+interface RIGHT<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    public B foo(A ChangeMe);
+}
diff --git a/t/t4018/java-interface-type-parameters-extends b/t/t4018/java-interface-type-parameters-extends
new file mode 100644
index 0000000000..31d7fb3244
--- /dev/null
+++ b/t/t4018/java-interface-type-parameters-extends
@@ -0,0 +1,6 @@
+interface RIGHT<A, B> extends Function<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    public B foo(A ChangeMe);
+}
diff --git a/userdiff.c b/userdiff.c
index d71b82feb7..bc5f3ed4c3 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -171,7 +171,7 @@ PATTERNS("html",
 PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
 	 /* Class, enum, and interface declarations */
-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*[ \t]+.*)$\n"
+	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+.*)$\n"
 	 /* Method definitions; note that constructor signatures are not */
 	 /* matched because they are indistinguishable from method calls. */
 	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 2/3] userdiff: support Java record types
  2023-02-07 23:42           ` [PATCH v3 " Andrei Rybak
  2023-02-07 23:42             ` [PATCH v3 1/3] userdiff: support Java type parameters Andrei Rybak
@ 2023-02-07 23:42             ` Andrei Rybak
  2023-02-07 23:42             ` [PATCH v3 3/3] userdiff: support Java sealed classes Andrei Rybak
  2023-02-08 20:51             ` [PATCH v3 0/3] userdiff: Java updates Johannes Sixt
  3 siblings, 0 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-07 23:42 UTC (permalink / raw)
  To: Johannes Sixt, git; +Cc: Jeff King, Paolo Bonzini, Tassilo Horn

A new kind of class was added in Java 16 -- records.[1]  The syntax of
records is similar to regular classes with one important distinction:
the name of the record class is followed by a mandatory list of
components.  The list is enclosed in parentheses, it may be empty, and
it may immediately follow the name of the class or type parameters, if
any, with or without separating whitespace.  For example:

    public record Example(int i, String s) {
    }

    public record WithTypeParameters<A, B>(A a, B b, String s) {
    }

    record SpaceBeforeComponents (String comp1, int comp2) {
    }

Support records in the builtin userdiff pattern for Java.  Add "record"
to the alternatives of keywords for kinds of class.

Allowing matching various possibilities for the type parameters and/or
list of the components of a record has already been covered by the
preceding patch.

[1] detailed description is available in "JEP 395: Records"
    https://openjdk.org/jeps/395

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t4018/java-record                         | 6 ++++++
 t/t4018/java-record-space-before-components | 6 ++++++
 t/t4018/java-record-type-parameters         | 6 ++++++
 userdiff.c                                  | 4 ++--
 4 files changed, 20 insertions(+), 2 deletions(-)
 create mode 100644 t/t4018/java-record
 create mode 100644 t/t4018/java-record-space-before-components
 create mode 100644 t/t4018/java-record-type-parameters

diff --git a/t/t4018/java-record b/t/t4018/java-record
new file mode 100644
index 0000000000..97aa819dd8
--- /dev/null
+++ b/t/t4018/java-record
@@ -0,0 +1,6 @@
+public record RIGHT(int comp1, double comp2, String comp3) {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    static int ChangeMe;
+}
diff --git a/t/t4018/java-record-space-before-components b/t/t4018/java-record-space-before-components
new file mode 100644
index 0000000000..9827f22583
--- /dev/null
+++ b/t/t4018/java-record-space-before-components
@@ -0,0 +1,6 @@
+public record RIGHT (String components, String after, String space) {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    static int ChangeMe;
+}
diff --git a/t/t4018/java-record-type-parameters b/t/t4018/java-record-type-parameters
new file mode 100644
index 0000000000..f62a035cc8
--- /dev/null
+++ b/t/t4018/java-record-type-parameters
@@ -0,0 +1,6 @@
+public record RIGHT<A, N extends Number>(A comp1, N comp2, int comp3) {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    static int ChangeMe;
+}
diff --git a/userdiff.c b/userdiff.c
index bc5f3ed4c3..37ac98e177 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -170,8 +170,8 @@ PATTERNS("html",
 	 "[^<>= \t]+"),
 PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
-	 /* Class, enum, and interface declarations */
-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+.*)$\n"
+	 /* Class, enum, interface, and record declarations */
+	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface|record)[ \t]+.*)$\n"
 	 /* Method definitions; note that constructor signatures are not */
 	 /* matched because they are indistinguishable from method calls. */
 	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 3/3] userdiff: support Java sealed classes
  2023-02-07 23:42           ` [PATCH v3 " Andrei Rybak
  2023-02-07 23:42             ` [PATCH v3 1/3] userdiff: support Java type parameters Andrei Rybak
  2023-02-07 23:42             ` [PATCH v3 2/3] userdiff: support Java record types Andrei Rybak
@ 2023-02-07 23:42             ` Andrei Rybak
  2023-02-08 20:51             ` [PATCH v3 0/3] userdiff: Java updates Johannes Sixt
  3 siblings, 0 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-07 23:42 UTC (permalink / raw)
  To: Johannes Sixt, git; +Cc: Jeff King, Paolo Bonzini, Tassilo Horn

A new kind of class was added in Java 17 -- sealed classes.[1]  This
feature includes several new keywords that may appear in a declaration
of a class.  New modifiers before name of the class: "sealed" and
"non-sealed", and a clause after name of the class marked by keyword
"permits".

The current set of regular expressions in userdiff.c already allows the
modifier "sealed" and the "permits" clause, but not the modifier
"non-sealed", which is the first hyphenated keyword in Java.[2]  Allow
hyphen in the words that precede the name of type to match the
"non-sealed" modifier.

In new input file "java-sealed" for the test t4018-diff-funcname.sh, use
a Java code comment for the marker "RIGHT".  This workaround is needed,
because the name of the sealed class appears on the line of code that
has the "ChangeMe" marker.

[1] Detailed description in "JEP 409: Sealed Classes"
    https://openjdk.org/jeps/409
[2] "JEP draft: Keyword Management for the Java Language"
    https://openjdk.org/jeps/8223002

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t4018/java-non-sealed                                | 8 ++++++++
 t/t4018/java-sealed                                    | 7 +++++++
 t/t4018/java-sealed-permits                            | 6 ++++++
 t/t4018/java-sealed-type-parameters                    | 6 ++++++
 t/t4018/java-sealed-type-parameters-implements-permits | 6 ++++++
 t/t4018/java-sealed-type-parameters-permits            | 6 ++++++
 userdiff.c                                             | 2 +-
 7 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/java-non-sealed
 create mode 100644 t/t4018/java-sealed
 create mode 100644 t/t4018/java-sealed-permits
 create mode 100644 t/t4018/java-sealed-type-parameters
 create mode 100644 t/t4018/java-sealed-type-parameters-implements-permits
 create mode 100644 t/t4018/java-sealed-type-parameters-permits

diff --git a/t/t4018/java-non-sealed b/t/t4018/java-non-sealed
new file mode 100644
index 0000000000..069087c1c6
--- /dev/null
+++ b/t/t4018/java-non-sealed
@@ -0,0 +1,8 @@
+public abstract sealed class SealedClass {
+    public static non-sealed class RIGHT extends SealedClass {
+        static int ONE;
+        static int TWO;
+        static int THREE;
+        private int ChangeMe;
+    }
+}
diff --git a/t/t4018/java-sealed b/t/t4018/java-sealed
new file mode 100644
index 0000000000..785fbc62bc
--- /dev/null
+++ b/t/t4018/java-sealed
@@ -0,0 +1,7 @@
+public abstract sealed class Sealed { // RIGHT
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    public final class ChangeMe extends Sealed {
+    }
+}
diff --git a/t/t4018/java-sealed-permits b/t/t4018/java-sealed-permits
new file mode 100644
index 0000000000..18dd4894cf
--- /dev/null
+++ b/t/t4018/java-sealed-permits
@@ -0,0 +1,6 @@
+public abstract sealed class RIGHT permits PermittedA, PermittedB {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/t/t4018/java-sealed-type-parameters b/t/t4018/java-sealed-type-parameters
new file mode 100644
index 0000000000..e6530c47c3
--- /dev/null
+++ b/t/t4018/java-sealed-type-parameters
@@ -0,0 +1,6 @@
+public abstract sealed class RIGHT<A, B> {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/t/t4018/java-sealed-type-parameters-implements-permits b/t/t4018/java-sealed-type-parameters-implements-permits
new file mode 100644
index 0000000000..bd6e6d3582
--- /dev/null
+++ b/t/t4018/java-sealed-type-parameters-implements-permits
@@ -0,0 +1,6 @@
+public abstract sealed class RIGHT<A, B> implements List<A> permits PermittedA, PermittedB {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/t/t4018/java-sealed-type-parameters-permits b/t/t4018/java-sealed-type-parameters-permits
new file mode 100644
index 0000000000..25a0da6442
--- /dev/null
+++ b/t/t4018/java-sealed-type-parameters-permits
@@ -0,0 +1,6 @@
+public abstract sealed class RIGHT<A, B> permits PermittedA, PermittedB {
+    static int ONE;
+    static int TWO;
+    static int THREE;
+    private int ChangeMe;
+}
diff --git a/userdiff.c b/userdiff.c
index 37ac98e177..94cca1a2a8 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -171,7 +171,7 @@ PATTERNS("html",
 PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
 	 /* Class, enum, interface, and record declarations */
-	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface|record)[ \t]+.*)$\n"
+	 "^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t]+.*)$\n"
 	 /* Method definitions; note that constructor signatures are not */
 	 /* matched because they are indistinguishable from method calls. */
 	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/3] userdiff: support Java type parameters
  2023-02-07 23:42             ` [PATCH v3 1/3] userdiff: support Java type parameters Andrei Rybak
@ 2023-02-08  0:04               ` Andrei Rybak
  0 siblings, 0 replies; 19+ messages in thread
From: Andrei Rybak @ 2023-02-08  0:04 UTC (permalink / raw)
  To: Johannes Sixt, git; +Cc: Jeff King, Paolo Bonzini, Tassilo Horn

On 2023-02-08T00:42, Andrei Rybak wrote:
> A class or interface in Java can have type parameters following the name
> in the declared type, surrounded by angle brackets (paired less than and
> greater than signs).[2]   The type parameters -- `A` and `B` in the
> examples -- may follow the class name immediately:
> 
>      public class ParameterizedClass<A, B> {
>      }
> 
> or may be separated by whitespace:
> 
>      public class SpaceBeforeTypeParameters <A, B> {
>      }
> 
> A part of the builtin userdiff pattern for Java matches declarations of
> classes, enums, and interfaces.  The regular expression requires at
> least one whitespace character after the name of the declared type.
> This disallows matching for opening angle bracket of type parameters
> immediately after the name of the type.  Mandatory whitespace after the
> name of the type also disallows using the pattern in repositories with a
> fairly common code style that puts braces for the body of a class on
> separate lines:
> 
>      class WithLineBreakBeforeOpeningBrace
>      {
>      }
> 
> Support matching Java code in more diverse code styles and declarations
> of classes and interfaces with type parameters immediately following the
> name of the type in the builtin userdiff pattern for Java.  Do so by
> just matching anything until the end of the line after the keywords for
> the kind of type being declared.

The above explains why removing the mandatory matching for whitespace
after the class name is needed, but it doesn't explain why removing
the part of the regex that matches the class name itself is OK.
Perhaps, something like this could be added:

     An possible approach could be to keep matching the name of the
     type: "...[ \t]+[A-Za-z][A-Za-z0-9_$]*.*)$\n", but without matching
     mandatory whitespace after the name of the type, matching the name
     itself separately isn't useful for our purposes.

?

> [1] Since Java 5 released in 2004.
> [2] Detailed description is available in the Java Language
>      Specification, sections "Type Variables" and "Parameterized Types":
>      https://docs.oracle.com/javase/specs/jls/se17/html/jls-4.html#jls-4.4
> 
> Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
> ---

[...]

> diff --git a/userdiff.c b/userdiff.c
> index d71b82feb7..bc5f3ed4c3 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -171,7 +171,7 @@ PATTERNS("html",
>   PATTERNS("java",
>   	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
>   	 /* Class, enum, and interface declarations */
> -	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+[A-Za-z][A-Za-z0-9_$]*[ \t]+.*)$\n"
> +	 "^[ \t]*(([a-z]+[ \t]+)*(class|enum|interface)[ \t]+.*)$\n"
>   	 /* Method definitions; note that constructor signatures are not */
>   	 /* matched because they are indistinguishable from method calls. */
>   	 "^[ \t]*(([A-Za-z_<>&][][?&<>.,A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 0/3] userdiff: Java updates
  2023-02-07 23:42           ` [PATCH v3 " Andrei Rybak
                               ` (2 preceding siblings ...)
  2023-02-07 23:42             ` [PATCH v3 3/3] userdiff: support Java sealed classes Andrei Rybak
@ 2023-02-08 20:51             ` Johannes Sixt
  2023-02-08 20:55               ` Junio C Hamano
  3 siblings, 1 reply; 19+ messages in thread
From: Johannes Sixt @ 2023-02-08 20:51 UTC (permalink / raw)
  To: Andrei Rybak; +Cc: Jeff King, Paolo Bonzini, Tassilo Horn, git

Am 08.02.23 um 00:42 schrieb Andrei Rybak:
> On 2023-02-05T22:33 Johannes Sixt wrote:
>> Having seen all these examples, I think the following truncated
>> expression might do the right thing for all cases that are valid Java:
>>
>> "^[ \t]*(([a-z-]+[ \t]+)*(class|enum|interface|record)[ \t].*)$"
> 
> Only the '\n' is missing at the end, but otherwise I concur, so here's a v3.
> 
>> i.e., we recognize a whitespace in order to identify the keyword, and
>> then capture anything that follows without being specific. My reasoning
>> is that "class", "enum", "interface", and "record" cannot occur in any
>> other context than the beginning of a class definition. (But please do
>> correct me; I know next to nothing about Java syntax.)
> 
> The word "class" can also occur as part of a class literal, for example:
> 
>     Class<String> c = String.class;
> 
> but valid uses of class literals won't interfere with our regex, unless some
> wild formatting is applied.  This is technically valid Java:
> 
>     Class<String> c = String.
>     class 
>     ;
> 
> and with a space after lowercase "class", the v3 regex will trip.

Yeah, let's assume that nobody writes code like this.

This iteration is all good!

Reviewed-by: Johannes Sixt <j6t@kdbg.org>

-- Hannes


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 0/3] userdiff: Java updates
  2023-02-08 20:51             ` [PATCH v3 0/3] userdiff: Java updates Johannes Sixt
@ 2023-02-08 20:55               ` Junio C Hamano
  0 siblings, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2023-02-08 20:55 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Andrei Rybak, Jeff King, Paolo Bonzini, Tassilo Horn, git

Johannes Sixt <j6t@kdbg.org> writes:

> Yeah, let's assume that nobody writes code like this.
>
> This iteration is all good!
>
> Reviewed-by: Johannes Sixt <j6t@kdbg.org>

Thanks.  I think I've queued this round already, but let me amend
your Reviewed-by into them.


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-02-08 20:56 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-03 23:23 [PATCH v1 0/3] userdiff: Java updates Andrei Rybak
2023-02-03 23:23 ` [PATCH v1 1/3] userdiff: support Java type parameters Andrei Rybak
2023-02-03 23:23 ` [PATCH v1 2/3] userdiff: support Java record types Andrei Rybak
2023-02-03 23:23 ` [PATCH v1 3/3] userdiff: support Java sealed classes Andrei Rybak
2023-02-04  9:22 ` [PATCH v1 0/3] userdiff: Java updates Tassilo Horn
2023-02-04 13:43   ` [PATCH v2 " Andrei Rybak
2023-02-04 13:43     ` [PATCH v2 1/3] userdiff: support Java type parameters Andrei Rybak
2023-02-04 13:43     ` [PATCH v2 2/3] userdiff: support Java record types Andrei Rybak
2023-02-04 13:43     ` [PATCH v2 3/3] userdiff: support Java sealed classes Andrei Rybak
2023-02-05 10:09     ` [PATCH v2 0/3] userdiff: Java updates Johannes Sixt
2023-02-05 19:27       ` Andrei Rybak
2023-02-05 21:33         ` Johannes Sixt
2023-02-07 23:42           ` [PATCH v3 " Andrei Rybak
2023-02-07 23:42             ` [PATCH v3 1/3] userdiff: support Java type parameters Andrei Rybak
2023-02-08  0:04               ` Andrei Rybak
2023-02-07 23:42             ` [PATCH v3 2/3] userdiff: support Java record types Andrei Rybak
2023-02-07 23:42             ` [PATCH v3 3/3] userdiff: support Java sealed classes Andrei Rybak
2023-02-08 20:51             ` [PATCH v3 0/3] userdiff: Java updates Johannes Sixt
2023-02-08 20:55               ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).