Fix three Flymake bugs when checking C header files

The first of these problems is longstanding: if an error-less B.h is
included from error-ridden A.h, flymake's legacy parser will panic
(and disable itself) since it sees a non-zero exit for a clean file.
To fix this, recommend returning 'true' in the documentation for the
check-syntax target.

Another problem was introduced by the parser rewrite.  For error
patterns spanning more than one line, point may be left in the middle
of a line and thus render other patterns useless.  Those patterns were
written for the old line-by-line parser.  To make them useful again,
move to the beginning of line in those situations.

The third problem was also longstanding and happened on newer GCC's:
The "In file included from" prefix confused
flymake-proc-get-real-file-name.  Fix this.

Also updated flymake--diag-region to fallback to highlighting a full
line less often.

Add automatic tests to check this.

* lisp/progmodes/flymake-proc.el
(flymake-proc--diagnostics-for-pattern): Fix bug when patterns
accidentally spans more than one line.  Don't create
diagnostics without error messages.
(flymake-proc-real-file-name-considering-includes): New
helper.
(flymake-proc-allowed-file-name-masks): Use it.

* lisp/progmodes/flymake.el (flymake-diag-region): Make COL
argument explicitly optional.  Only fall back to full line in extreme
cases.

* test/lisp/progmodes/flymake-tests.el
(included-c-header-files): New test.
(different-diagnostic-types): Update.

* test/lisp/progmodes/flymake-resources/Makefile
(check-syntax): Always return success (0) error code.
(CC_OPTS): Add -Wextra

* test/lisp/progmodes/flymake-resources/errors-and-warnings.c
(main): Rewrite comments.

* test/lisp/progmodes/flymake-resources/errors-and-warnings.c:
Include some dummy header files.

* test/lisp/progmodes/flymake-resources/no-problems.h: New file.

* test/lisp/progmodes/flymake-resources/some-problems.h: New file.

* doc/misc/flymake.texi (Example---Configuring a tool called
via make): Recommend adding "|| true" to the check-syntax target.

8 files changed, 81 insertions, 23 deletions

diff --git a/doc/misc/flymake.texi b/doc/misc/flymake.texi
index 1bc416fd02e..01849b7d9a5 100644
--- a/doc/misc/flymake.texi
+++ b/doc/misc/flymake.texi
@@ -492,7 +492,7 @@ our case this target might look like this:
 
 @verbatim
 check-syntax:
-	gcc -o /dev/null -S ${CHK_SOURCES}
+	gcc -o /dev/null -S ${CHK_SOURCES} || true
 @end verbatim
 
 @noindent
@@ -504,7 +504,7 @@ Automake variable @code{COMPILE}:
 
 @verbatim
 check-syntax:
-	$(COMPILE) -o /dev/null -S ${CHK_SOURCES}
+	$(COMPILE) -o /dev/null -S ${CHK_SOURCES} || true
 @end verbatim
 
 @node Flymake Implementation
diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index 37b7e49dea4..48d35598b72 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -66,7 +66,10 @@
   :type 'integer)
 
 (defcustom flymake-proc-allowed-file-name-masks
-  '(("\\.\\(?:c\\(?:pp\\|xx\\|\\+\\+\\)?\\|CC\\)\\'" flymake-proc-simple-make-init)
+  '(("\\.\\(?:c\\(?:pp\\|xx\\|\\+\\+\\)?\\|CC\\)\\'"
+     flymake-proc-simple-make-init
+     nil
+     flymake-proc-real-file-name-considering-includes)
     ("\\.xml\\'" flymake-proc-xml-init)
     ("\\.html?\\'" flymake-proc-xml-init)
     ("\\.cs\\'" flymake-proc-simple-make-init)
@@ -419,12 +422,25 @@ Create parent directories as needed."
     (condition-case-unless-debug err
         (cl-loop
          with (regexp file-idx line-idx col-idx message-idx) = pattern
-         while (search-forward-regexp regexp nil t)
+         while (and
+                (search-forward-regexp regexp nil t)
+                ;; If the preceding search spanned more than one line,
+                ;; move to the start of the line we ended up in. This
+                ;; preserves the usefulness of the patterns in
+                ;; `flymake-proc-err-line-patterns', which were
+                ;; written primarily for flymake's original
+                ;; line-by-line parsing and thus never spanned
+                ;; multiple lines.
+                (if (/= (line-number-at-pos (match-beginning 0))
+                        (line-number-at-pos))
+                    (goto-char (line-beginning-position))
+                  t))
          for fname = (and file-idx (match-string file-idx))
          for message = (and message-idx (match-string message-idx))
          for line-string = (and line-idx (match-string line-idx))
-         for line-number = (and line-string
-                                (string-to-number line-string))
+         for line-number = (or (and line-string
+                                    (string-to-number line-string))
+                               1)
          for col-string = (and col-idx (match-string col-idx))
          for col-number = (and col-string
                                (string-to-number col-string))
@@ -436,7 +452,7 @@ Create parent directories as needed."
                                  fname)))
          for buffer = (and full-file
                            (find-buffer-visiting full-file))
-         if (eq buffer (process-buffer proc))
+         if (and (eq buffer (process-buffer proc)) message)
          collect (with-current-buffer buffer
                    (pcase-let ((`(,beg . ,end)
                                 (flymake-diag-region line-number col-number)))
@@ -1030,6 +1046,13 @@ Use CREATE-TEMP-F for creating temp copy."
    '("\\.\\(?:c\\(?:pp\\|xx\\|\\+\\+\\)?\\|CC\\)\\'")
    "[ \t]*#[ \t]*include[ \t]*\"\\([[:word:]0-9/\\_.]*%s\\)\""))
 
+(defun flymake-proc-real-file-name-considering-includes (scraped)
+  (flymake-proc-get-real-file-name
+   (let ((case-fold-search t))
+     (replace-regexp-in-string "^in file included from[ \t*]"
+                               ""
+                               scraped))))
+
 ;;;; .java/make specific
 (defun flymake-proc-simple-make-java-init ()
   (flymake-proc-simple-make-init-impl 'flymake-proc-create-temp-with-folder-structure nil nil "Makefile" 'flymake-proc-get-make-cmdline))
diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 282727e315d..285ef93db62 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -248,9 +248,10 @@ verify FILTER, sort them by COMPARE (using KEY)."
 (define-obsolete-face-alias 'flymake-warnline 'flymake-warning "26.1")
 (define-obsolete-face-alias 'flymake-errline 'flymake-error "26.1")
 
-(defun flymake-diag-region (line col)
+(defun flymake-diag-region (line &optional col)
   "Compute region (BEG . END) corresponding to LINE and COL.
-Or nil if the region is invalid."
+If COL is nil, return a region just for LINE.
+Return nil if the region is invalid."
   (condition-case-unless-debug _err
       (let ((line (min (max line 1)
                        (line-number-at-pos (point-max) 'absolute))))
@@ -267,13 +268,18 @@ Or nil if the region is invalid."
                        (if (eq (point) beg)
                            (line-beginning-position 2)
                          (point)))))
-            (if col
-                (let* ((beg (progn (forward-char (1- col)) (point)))
+            (if (and col (cl-plusp col))
+                (let* ((beg (progn (forward-char (1- col))
+                                   (point)))
                        (sexp-end (ignore-errors (end-of-thing 'sexp)))
-                       (end (or sexp-end
-                                (fallback-eol beg))))
-                  (cons (if sexp-end beg (fallback-bol))
-                        end))
+                       (end (or (and sexp-end
+                                     (not (= sexp-end beg))
+                                     sexp-end)
+                                (ignore-errors (goto-char (1+ beg)))))
+                       (safe-end (or end
+                                     (fallback-eol beg))))
+                  (cons (if end beg (fallback-bol))
+                        safe-end))
               (let* ((beg (fallback-bol))
                      (end (fallback-eol beg)))
                 (cons beg end))))))
diff --git a/test/lisp/progmodes/flymake-resources/Makefile b/test/lisp/progmodes/flymake-resources/Makefile
index 0f3f39791c8..494407567f2 100644
--- a/test/lisp/progmodes/flymake-resources/Makefile
+++ b/test/lisp/progmodes/flymake-resources/Makefile
@@ -1,6 +1,6 @@
 # Makefile for flymake tests
 
-CC_OPTS = -Wall
+CC_OPTS = -Wall -Wextra
 
 ## Recent gcc (e.g. 4.8.2 on RHEL7) can automatically colorize their output,
 ## which can confuse flymake.  Set GCC_COLORS to disable that.
@@ -8,6 +8,6 @@ CC_OPTS = -Wall
 ## normally use flymake, so it seems like just avoiding the issue
 ## in this test is fine.  Set flymake-log-level to 3 to investigate.
 check-syntax:
-	GCC_COLORS= $(CC) $(CC_OPTS) ${CHK_SOURCES}
+	GCC_COLORS= $(CC) $(CC_OPTS) ${CHK_SOURCES} || true
 
 # eof
diff --git a/test/lisp/progmodes/flymake-resources/errors-and-warnings.c b/test/lisp/progmodes/flymake-resources/errors-and-warnings.c
index 6454dd20236..1d38bd6bd27 100644
--- a/test/lisp/progmodes/flymake-resources/errors-and-warnings.c
+++ b/test/lisp/progmodes/flymake-resources/errors-and-warnings.c
@@ -1,10 +1,13 @@
- int main()
+/* Flymake should notice an error on the next line, since
+   that file has at least one warning.*/
+#include "some-problems.h"
+/* But not this one */
+#include "no-problems.h"
+
+int main()
 {
-  char c = 1000;
+  char c = 1000; /* a note and a warning */
   int bla;
-  /* The following line should have one warning and one error. The
-     warning spans the full line because gcc (at least 6.3.0) points
-     places the error at the =, which isn't a sexp.*/
-  char c; if (bla == (void*)3);
+  char c; if (bla == (void*)3); /* an error, and two warnings */
   return c;
 }
diff --git a/test/lisp/progmodes/flymake-resources/no-problems.h b/test/lisp/progmodes/flymake-resources/no-problems.h
new file mode 100644
index 00000000000..19ddc615b32
--- /dev/null
+++ b/test/lisp/progmodes/flymake-resources/no-problems.h
@@ -0,0 +1 @@
+typedef int no_problems;
diff --git a/test/lisp/progmodes/flymake-resources/some-problems.h b/test/lisp/progmodes/flymake-resources/some-problems.h
new file mode 100644
index 00000000000..165d8dd525e
--- /dev/null
+++ b/test/lisp/progmodes/flymake-resources/some-problems.h
@@ -0,0 +1,5 @@
+#include <stdio.h>
+
+strange;
+
+sint main();
diff --git a/test/lisp/progmodes/flymake-tests.el b/test/lisp/progmodes/flymake-tests.el
index fa77a9a8ae6..222c8f11848 100644
--- a/test/lisp/progmodes/flymake-tests.el
+++ b/test/lisp/progmodes/flymake-tests.el
@@ -122,14 +122,34 @@ SEVERITY-PREDICATE is used to setup
   (flymake-tests--with-flymake
       ("errors-and-warnings.c")
     (flymake-goto-next-error)
+    (should (eq 'flymake-error (face-at-point)))
+    (flymake-goto-next-error)
     (should (eq 'flymake-note (face-at-point)))
     (flymake-goto-next-error)
     (should (eq 'flymake-warning (face-at-point)))
     (flymake-goto-next-error)
+    (should (eq 'flymake-error (face-at-point)))
+    (flymake-goto-next-error)
+    (should (eq 'flymake-warning (face-at-point)))
+    (flymake-goto-next-error)
+    (should (eq 'flymake-warning (face-at-point)))
+    (let ((flymake-wrap-around nil))
+      (should-error (flymake-goto-next-error nil nil t))) ))
+
+(ert-deftest included-c-header-files ()
+  "Test inclusion of .h header files."
+  (skip-unless (and (executable-find "gcc") (executable-find "make")))
+  (flymake-tests--with-flymake
+      ("some-problems.h")
+    (flymake-goto-next-error)
     (should (eq 'flymake-warning (face-at-point)))
     (flymake-goto-next-error)
     (should (eq 'flymake-error (face-at-point)))
     (let ((flymake-wrap-around nil))
+      (should-error (flymake-goto-next-error nil nil t))) )
+  (flymake-tests--with-flymake
+      ("no-problems.h")
+    (let ((flymake-wrap-around nil))
       (should-error (flymake-goto-next-error nil nil t))) ))
 
 (defmacro flymake-tests--assert-set (set