procyberian/emacs
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix three Flymake bugs when checking C header files

Browse source 
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.
This commit is contained in:
João Távora 2017-09-28 12:06:56 +01:00
parent 9a629a73e0
commit 8118f0f95f
8 changed files with 81 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
doc/misc/flymake.texi
View file

@ -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

33
lisp/progmodes/flymake-proc.el
View file

@ -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))

22
lisp/progmodes/flymake.el
View file

@ -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))))))

4
test/lisp/progmodes/flymake-resources/Makefile
View file

@ -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

15
test/lisp/progmodes/flymake-resources/errors-and-warnings.c
View file

@ -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;
}

1
test/lisp/progmodes/flymake-resources/no-problems.h Normal file
View file

@ -0,0 +1 @@
typedef int no_problems;

5
test/lisp/progmodes/flymake-resources/some-problems.h Normal file
View file

@ -0,0 +1,5 @@
#include <stdio.h>
strange;
sint main();

20
test/lisp/progmodes/flymake-tests.el
View file

@ -122,13 +122,33 @@ 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))) ))