userdiff: improve java regex for generic return types

Commit Message

Tassilo Horn Aug. 9, 2021, 8:33 p.m. UTC

Currently, the git diff hunk headers show the wrong method signature if the
method has a generic return type because the regex doesn't allow < and > in the
return type.  This patch adds those.

Bug repro: In a repository with .gitattributes containing "*.java diff=java"
and a java file

--8<---------------cut here---------------start------------->8---
class MyExample {
    public void firstMethod() {
        // Whatever...
    }

    public List<Integer> secondMethod() {
        // Here is some comment,
        // and here is more,
        // and here is even more,
        // followed by this,
        // and that,
        // and even more.
        return Arrays.asList(1, 2, 3, 4, 5);
    }
}
--8<---------------cut here---------------end--------------->8---

when adding the number 6 to the Arrays.asList() call in secondMethod(), the git

Comments

Jeff King Aug. 9, 2021, 9 p.m. UTC | #1

On Mon, Aug 09, 2021 at 10:33:08PM +0200, Tassilo Horn wrote:

> Currently, the git diff hunk headers show the wrong method signature if the
> method has a generic return type because the regex doesn't allow < and > in the
> return type.  This patch adds those.

Thanks. It has been long enough since I used Java that there were no
generics back then, but I will take your word that this is what they
look like. ;)

The patch itself looks OK to me, but...

> when adding the number 6 to the Arrays.asList() call in secondMethod(), the git
> diff one gets is
> 
> --8<---------------cut here---------------start------------->8---
> diff --git a/src/main/java/MyExample.java b/src/main/java/MyExample.java
> index a0f1a6b..ea37a98 100644
> --- a/src/main/java/MyExample.java
> +++ b/src/main/java/MyExample.java
> @@ -14,6 +14,6 @@ public void firstMethod() {
>          // followed by this,
>          // and that,
>          // and even more.
> -        return Arrays.asList(1, 2, 3, 4, 5);
> +        return Arrays.asList(1, 2, 3, 4, 5, 6);
>      }
>  }
> --8<---------------cut here---------------end--------------->8---

...this diff in the commit message will screw up git-am. The usual
procedure is to omit the scissors lines and just indent it.

But...

>  userdiff.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

...even better than explaining it is adding a regression test, perhaps
like this one:

diff --git a/t/t4018/java-return-generic b/t/t4018/java-return-generic
new file mode 100644
index 0000000000..d030c26184
--- /dev/null
+++ b/t/t4018/java-return-generic
@@ -0,0 +1,10 @@
+class MyExample {
+    public void firstMethod() {
+        // Whatever...
+    }
+
+    public List<String> secondMethod(String RIGHT[]) {
+        // Whatever...
+        return Arrays.asList("ChangeMe");
+    }
+}

(the "ChangeMe" line will be changed, and we'll expect that the line
with "RIGHT" on it is found. We use -U1 to reduce the need for filler).

-Peff

Tassilo Horn Aug. 10, 2021, 4:51 a.m. UTC | #2

Jeff King <[e-mail]> writes:

Hi Jeff,

> The patch itself looks OK to me, but...

thanks for the detailed comments.  I'll submit a new version of the
patch soon.

Bye,
Tassilo

Patch

diff one gets is

--8<---------------cut here---------------start------------->8---
diff --git a/src/main/java/MyExample.java b/src/main/java/MyExample.java
index a0f1a6b..ea37a98 100644
--- a/src/main/java/MyExample.java
+++ b/src/main/java/MyExample.java
@@ -14,6 +14,6 @@  public void firstMethod() {
         // followed by this,
         // and that,
         // and even more.
-        return Arrays.asList(1, 2, 3, 4, 5);
+        return Arrays.asList(1, 2, 3, 4, 5, 6);
     }
 }
--8<---------------cut here---------------end--------------->8---

where the hunk header shows the signature of firstMethod whereas it should show
the signature of secondMethod.

Signed-off-by: Tassilo Horn <[e-mail]>
---
 userdiff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/userdiff.c b/userdiff.c
index 3c3bbe38b0..a244ad7ab1 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -142,7 +142,7 @@  PATTERNS("html",
 	 "[^<>= \t]+"),
 PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
-	 "^[ \t]*(([A-Za-z_][A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
+	 "^[ \t]*(([A-Za-z_][A-Za-z_0-9<>]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ \t]*\\([^;]*)$",
 	 /* -- */
 	 "[a-zA-Z_][a-zA-Z0-9_]*"
 	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"