Review request (compilation-error-regexp-alist-alist)

Review request (compilation-error-regexp-alist-alist)

[Filipp Gunbin]

Hi, could you please review this to check I haven't done anything
obviously wrong?

Thanks.
Filipp

diff --git a/etc/compilation.txt b/etc/compilation.txt
index 0e39ab5e4a..4a4a318d03 100644
--- a/etc/compilation.txt
+++ b/etc/compilation.txt
@@ -185,6 +185,14 @@ Warning near line 10 file arrayclash.f: Module contains no executable
 Nonportable usage near line 31 col 9 file assign.f: mixed default and explicit
 
 
+* Gradle with kotlin-gradle-plugin
+
+symbol: gradle-kotlin
+
+e: /src/Test.kt: (34, 15): foo: bar
+w: /src/Test.kt: (34, 15): foo: bar
+
+
 * IAR Systems C Compiler
 
 symbol: iar
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index cd7a5dc677..24c8a2a312 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -236,6 +236,15 @@ compilation-error-regexp-alist-alist
      "\\(^Warning .*\\)? line[ \n]\\([0-9]+\\)[ \n]\\(?:col \\([0-9]+\\)[ \n]\\)?file \\([^ :;\n]+\\)"
      4 2 3 (1))
 
+    ;; Gradle with kotlin-gradle-plugin (see
+    ;; GradleStyleMessagerRenderer.kt in kotlin sources).
+    (gradle-kotlin
+     ,(concat
+       "^\\(?:\\(w\\)\\|.\\): *"            ;type
+       "\\(\\(?:[A-Za-z]:\\)?[^:\n]+\\): *" ;file
+       "(\\([0-9]+\\), *\\([0-9]+\\))")     ;line, column
+     2 3 4 (1))
+
     (iar
      "^\"\\(.*\\)\",\\([0-9]+\\)\\s-+\\(?:Error\\|Warnin\\(g\\)\\)\\[[0-9]+\\]:"
      1 2 nil (3))
diff --git a/test/lisp/progmodes/compile-tests.el b/test/lisp/progmodes/compile-tests.el
index 08a369e7b5..e38c31dd0a 100644
--- a/test/lisp/progmodes/compile-tests.el
+++ b/test/lisp/progmodes/compile-tests.el
@@ -212,6 +212,13 @@ compile-tests--test-regexps-data
      1 nil 27041 "{standard input}")
     ("boost/container/detail/flat_tree.hpp:589:25:   [ skipping 5 instantiation contexts, use -ftemplate-backtrace-limit=0 to disable ]"
      1 25 589 "boost/container/detail/flat_tree.hpp" 0)
+    ;; gradle-kotlin
+    ("e: /src/Test.kt: (34, 15): foo: bar" 4 15 34 "/src/Test.kt" 2)
+    ("w: /src/Test.kt: (11, 98): foo: bar" 4 98 11 "/src/Test.kt" 1)
+    ("e: e:/cygwin/src/Test.kt: (34, 15): foo: bar" 4 15 34 "e:/cygwin/src/Test.kt" 2)
+    ("w: e:/cygwin/src/Test.kt: (11, 98): foo: bar" 4 98 11 "e:/cygwin/src/Test.kt" 1)
+    ("e: e:\\src\\Test.kt: (34, 15): foo: bar" 4 15 34 "e:\\src\\Test.kt" 2)
+    ("w: e:\\src\\Test.kt: (11, 98): foo: bar" 4 98 11 "e:\\src\\Test.kt" 1)
     ;; Guile
     ("In foo.scm:\n" 1 nil nil "foo.scm")
     ("  63:4 [call-with-prompt prompt0 ...]" 1 4 63 nil)
@@ -413,8 +420,8 @@ compile--test-error-line
           (compilation-num-warnings-found 0)
           (compilation-num-infos-found 0))
       (mapc #'compile--test-error-line compile-tests--test-regexps-data)
-      (should (eq compilation-num-errors-found 87))
-      (should (eq compilation-num-warnings-found 32))
+      (should (eq compilation-num-errors-found 90))
+      (should (eq compilation-num-warnings-found 35))
       (should (eq compilation-num-infos-found 26)))))
 
 (ert-deftest compile-test-grep-regexps ()

Re: Review request (compilation-error-regexp-alist-alist)

[Stefan Monnier]

Hi,

> Hi, could you please review this to check I haven't done anything
> obviously wrong?

Could you open a bug report against Kotlin requesting they use
a standard error format instead of inventing yet another one?


        Stefan

Re: Review request (compilation-error-regexp-alist-alist)

[Filipp Gunbin]

Hi Stefan,

On 28/10/2019 17:43 -0400, Stefan Monnier wrote:

> Could you open a bug report against Kotlin requesting they use
> a standard error format instead of inventing yet another one?

Looks like it's done on purpose.  Current source (I refer to it in
comment in patch) is here:
https://github.com/JetBrains/kotlin/blob/master/compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/GradleStyleMessagerRenderer.kt

If I trace back a little, I see this commit made 5 years ago:
https://github.com/JetBrains/kotlin/commit/ffe8ae3840d7b9bdc82170c8181031f05ced68bd,
in which they explicitly set the present format.  I don't know why they
did that, and commit message doesn't help.

So probably it's not very promising to file a bug about it.

BTW, kotlin compiler by itself reports more "normal" error messages,
this one-letter-error-type format is for gradle plugin only.

The reason for my patch is that kotlin + gradle is a popular combination
nowadays in java world (not that I like this fact :-)  In this course I
also added basic support for gradle in my javaimp package (now on
scratch branch, will merge in elpa master soon).

Thanks,
Filipp

Re: Review request (compilation-error-regexp-alist-alist)

[Stefan Monnier]

> So probably it's not very promising to file a bug about it.

We can't know before we do it.


        Stefan

Re: Review request (compilation-error-regexp-alist-alist)

[Filipp Gunbin]

On 28/10/2019 21:13 -0400, Stefan Monnier wrote:

>> So probably it's not very promising to file a bug about it.
>
> We can't know before we do it.
>

The code and that commit are rather convincing (to me).

But I can try.  What can I refer to as a standard error format?

Filipp

Re: Review request (compilation-error-regexp-alist-alist)

[Stefan Monnier]

> But I can try.  What can I refer to as a standard error format?

I'd suggest https://www.gnu.org/prep/standards/html_node/Errors.html#Errors


        Stefan

Re: Review request (compilation-error-regexp-alist-alist)

[Filipp Gunbin]

On 29/10/2019 09:17 -0400, Stefan Monnier wrote:

>> But I can try.  What can I refer to as a standard error format?
>
> I'd suggest https://www.gnu.org/prep/standards/html_node/Errors.html#Errors

How one could include error type (error / warning / info) in this
format?

Filipp

Re: Review request (compilation-error-regexp-alist-alist)

[Stefan Monnier]

>>> But I can try.  What can I refer to as a standard error format?
>> I'd suggest https://www.gnu.org/prep/standards/html_node/Errors.html#Errors
> How one could include error type (error / warning / info) in this
> format?

Not sure about "info", but for "warning" it's usually done by placing
"Warning:" right at the beginning of the actual message (after the
file/line/col info).


        Stefan

Re: Review request (compilation-error-regexp-alist-alist)

[Filipp Gunbin]

On 29/10/2019 10:51 -0400, Stefan Monnier wrote:

>>>> But I can try.  What can I refer to as a standard error format?
>>> I'd suggest https://www.gnu.org/prep/standards/html_node/Errors.html#Errors
>> How one could include error type (error / warning / info) in this
>> format?
>
> Not sure about "info", but for "warning" it's usually done by placing
> "Warning:" right at the beginning of the actual message (after the
> file/line/col info).

Thanks!  Filed a report: https://youtrack.jetbrains.com/issue/KT-34683,
will post updates here.  Should I merge the patch, or wait for more
comments?

Filipp

Re: Review request (compilation-error-regexp-alist-alist)

[Stefan Monnier]

> Thanks!  Filed a report: https://youtrack.jetbrains.com/issue/KT-34683,
> will post updates here.  Should I merge the patch, or wait for more
> comments?

No objection in my side (but please include a link to
https://youtrack.jetbrains.com/issue/KT-34683 in the comments).


        Stefan

Re: Review request (compilation-error-regexp-alist-alist)

[Filipp Gunbin]

On 29/10/2019 13:14 -0400, Stefan Monnier wrote:

>> Thanks!  Filed a report: https://youtrack.jetbrains.com/issue/KT-34683,
>> will post updates here.  Should I merge the patch, or wait for more
>> comments?
>
> No objection in my side (but please include a link to
> https://youtrack.jetbrains.com/issue/KT-34683 in the comments).

Added, and pushed.  Thanks!
Filipp

Re: Review request (compilation-error-regexp-alist-alist)

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > If I trace back a little, I see this commit made 5 years ago:
  > https://github.com/JetBrains/kotlin/commit/ffe8ae3840d7b9bdc82170c8181031f05ced68bd,
  > in which they explicitly set the present format.  I don't know why they
  > did that, and commit message doesn't help.

It could be useful to ask them their reasons.  Then we could work with them
in looking for the best solution.  Perhaps they need to go beyond what our
standard has room for; if so, we could work with them to extend it.
We have extended it in the past.

-- 
Dr Richard Stallman
Founder, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: Review request (compilation-error-regexp-alist-alist)

[Stefan Monnier]

> It could be useful to ask them their reasons.  Then we could work with them
> in looking for the best solution.  Perhaps they need to go beyond what our
> standard has room for; if so, we could work with them to extend it.
> We have extended it in the past.

A clear need is to document how to specify the severity level.


        Stefan

Re: Review request (compilation-error-regexp-alist-alist)

[Filipp Gunbin]

Please review one more patch.

When gradle invokes ant tasks, the output seems to be like from plain
ant.  This patch makes ant regexp more lax:

- Accept filenames that may occur on Cygwin, like c:/test (in addition
  to c:\test)

- Accept optional additional severity level after task name.  Checkstyle
  - a popular ant task - outputs its own severity level.

I'd be grateful if someone could test it on Windows/Cygwin.  I used to
be a Cygwin user, but now don't have access to a Windows machine.

Thanks.

diff --git a/etc/compilation.txt b/etc/compilation.txt
index 4a4a318d03..7cd33bbd7a 100644
--- a/etc/compilation.txt
+++ b/etc/compilation.txt
@@ -49,10 +49,11 @@ The regexps found on http://ant.apache.org/faq.html, and since
 integrated in both Emacsen, were hairy.  The extra two numbers for
 jikes are the ending line and ending column.
 
-    [javac] /src/DataBaseTestCase.java:27: unreported exception ...
-    [javac] /src/DataBaseTestCase.java:49: warning: finally clause cannot complete normally
-    [jikes]  foo.java:3:5:7:9: blah blah
-  [javadoc] c:\MyProject\Polynomial.java:560: error: unknown tag: math
+       [javac] /src/DataBaseTestCase.java:27: unreported exception ...
+       [javac] /src/DataBaseTestCase.java:49: warning: finally clause cannot complete normally
+      [jikes]  foo.java:3:5:7:9: blah blah
+     [javadoc] c:\MyProject\Polynomial.java:560: error: unknown tag: math
+  [checkstyle] [ERROR] /src/Test.java:38: warning: foo: bar
 
 
 * Bash v2
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index b0bb728de0..f412dbcd54 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -181,9 +181,12 @@ compilation-error-regexp-alist-alist
     (aix
      " in line \\([0-9]+\\) of file \\([^ \n]+[^. \n]\\)\\.? " 2 1)
 
+    ;; Checkstyle task may report its own severity level: "[checkstyle] [ERROR] ..."
+    ;; (see AuditEventDefaultFormatter.java in checkstyle sources).
     (ant
-     "^[ \t]*\\[[^] \n]+\\][ \t]*\\(\\(?:[A-Za-z]:\\\\\\)?[^: \n]+\\):\\([0-9]+\\):\\(?:\\([0-9]+\\):\\([0-9]+\\):\\([0-9]+\\):\\)?\
-\\( warning\\)?" 1 (2 . 4) (3 . 5) (6))
+     "^[ \t]*\\(?:\\[[^] \n]+\\][ \t]*\\)\\{1,2\\}\\(\\(?:[A-Za-z]:\\)?[^: \n]+\\):\
+\\([0-9]+\\):\\(?:\\([0-9]+\\):\\([0-9]+\\):\\([0-9]+\\):\\)?\\( warning\\)?"
+     1 (2 . 4) (3 . 5) (6))
 
     (bash
      "^\\([^: \n\t]+\\): line \\([0-9]+\\):" 1 2)
diff --git a/test/lisp/progmodes/compile-tests.el b/test/lisp/progmodes/compile-tests.el
index e38c31dd0a..4c2ce83dde 100644
--- a/test/lisp/progmodes/compile-tests.el
+++ b/test/lisp/progmodes/compile-tests.el
@@ -55,11 +55,19 @@ compile-tests--test-regexps-data
      25 nil 8 "errors.c")
     ;; ant
     ("[javac] /src/DataBaseTestCase.java:27: unreported exception ..."
-     13 nil 27 "/src/DataBaseTestCase.java")
+     13 nil 27 "/src/DataBaseTestCase.java" 2)
     ("[javac] /src/DataBaseTestCase.java:49: warning: finally clause cannot complete normally"
-     13 nil 49 "/src/DataBaseTestCase.java")
+     13 nil 49 "/src/DataBaseTestCase.java" 1)
     ("[jikes]  foo.java:3:5:7:9: blah blah"
-     14 (5 . 10) (3 . 7) "foo.java")
+     14 (5 . 10) (3 . 7) "foo.java" 2)
+    ("[javac] c:/cygwin/Test.java:12: error: foo: bar"
+     9 nil 12 "c:/cygwin/Test.java" 2)
+    ("[javac] c:\\cygwin\\Test.java:87: error: foo: bar"
+     9 nil 87 "c:\\cygwin\\Test.java" 2)
+    ;; Checkstyle error, but ant reports a warning (note additional
+    ;; severity level after task name)
+    ("[checkstyle] [ERROR] /src/Test.java:38: warning: foo"
+     22 nil 38 "/src/Test.java" 1)
     ;; bash
     ("a.sh: line 1: ls-l: command not found"
      1 nil 1 "a.sh")
@@ -420,8 +428,8 @@ compile--test-error-line
           (compilation-num-warnings-found 0)
           (compilation-num-infos-found 0))
       (mapc #'compile--test-error-line compile-tests--test-regexps-data)
-      (should (eq compilation-num-errors-found 90))
-      (should (eq compilation-num-warnings-found 35))
+      (should (eq compilation-num-errors-found 92))
+      (should (eq compilation-num-warnings-found 36))
       (should (eq compilation-num-infos-found 26)))))
 
 (ert-deftest compile-test-grep-regexps ()

Re: Review request (compilation-error-regexp-alist-alist)

[Stefan Monnier]

> Please review one more patch.

No objection from my side,


        Stefan

Re: Review request (compilation-error-regexp-alist-alist)

[Filipp Gunbin]

Thanks, pushed.