jdee icon indicating copy to clipboard operation
jdee copied to clipboard

Flycheck does not display errors

Open nicholsonscn opened this issue 7 years ago • 3 comments

When running flycheck on a java file the errors show up in the compilation buffer, and in the messages buffer, but not the flycheck errors buffer nor any highlighting in the text buffer.

Tracking the issue down seems to be due to the flycheck error object instance that is created is populated with the file field containing the pathname to the temporary directory rather than the original file.

Jdee-flycheck.el has the code that collects the errors discovered by parsing the compiler output. One of the fields it (method: jdee-flycheck--collect-errors) sets is the name of the file. However, the problem appears to be that the file specified is the full path of the copy of the file in the temp directory.

A quick hack I did to verify was to change the file to be the original full path filename rather than the one from the regex (which is the temp file path). Once I did this everything started working (flycheck errors showed up in flycheck list and the buffer correctly marked the lines with issues).

I have not tested this when handling a project with multiple files with errors in different files and I suspect, looking at the code, this hack won’t work properly (since it doesn't filter compiler errors from other files - and my hack would trump them all to come from the current file) but it’s a start.

Versions: Windows: Windows 10 Pro Version: 1703 Build: 15063.413 Emacs: GNU Emacs 25.2.1 (x86_64-w64-mingw32) of 2017-04-24 JDEE: 2.4.2 (installed from MELPA) Java: jdk 1.8.0_131

nicholsonscn avatar Jul 02 '17 20:07 nicholsonscn

Hello, I have found the same problem. The problem is in this function

 (defun jdee-flycheck--restore-original-filename (temp-name orig-name)
 "Replace all occurances of TEMP-NAME with ORIG-NAME in current buffer."
  (goto-char (point-min))
   (while (re-search-forward temp-name nil t)
    (replace-match orig-name)))

Jdee-FlyCheck creates a copy of the java file in a Temp Directory. Then compiles it with javac and collect de errors from the stdout. Something like this:

c:\Users\xxxx\AppData\Local\Temp\JDEE_flycheck_18704lss\Test.java:13: error: <identifier> expected
    private Slal;
                ^
c:\Users\xxxx\AppData\Local\Temp\JDEE_flycheck_18704lss\Test.java:46: error: <identifier> expected
    private a;

Before parse the stdout to create FlyCheck errors, it searchs the temp file path and replaces it with the original file path. In windows, it doesn't works because de file separator. Emacs uses / and javac in windows uses . So it doesn't find any ocurrences of the temp file path. In previous example, it is looking for the c:/Users/aarteaga/AppData/Local/Temp/JDEE_flycheck_18704lss/Prueba.java:13.

Two Solutions:

  1. Set Enviroment var JAVA_TOOL_OPTIONS=-Dfile.separator=/ . This force all Java applications to use / instead of , included javac. This caused trouble to me, because breaks jdee-maven-build, but perhaps it works for you.

  2. Change jdee-flycheck.el. Replace the above function with this (I'm sure the code could be better, but i'm a really noob in Lisp and Emacs):

(defun jdee-flycheck--restore-original-filename (temp-name orig-name)
  "Replace all occurances of TEMP-NAME with ORIG-NAME in current buffer."
  (goto-char (point-min))
  (let ((temp-linux-name (replace-regexp-in-string  "\\\\" "/" temp-name))
		(temp-windows-name (replace-regexp-in-string  "/" "\\\\\\\\" temp-name)))
    (while (re-search-forward temp-linux-name nil t)
      (replace-match orig-name))
        (goto-char(point-min))
    (while (re-search-forward temp-windows-name nil t)
      (replace-match orig-name)))
  )

. This the solution that works nice for me.

Hope it Helps!!.

flush avatar Jul 05 '17 11:07 flush

Thank you that does help quite a bit.

I did go a slightly different route though. It seems wasteful to do a regex search/replacement over the entire temp buffer (for the pathname) and then go back and do another regex search over the buffer for the errors (which grabs out the pathname).

Now in your modified/fixed method that first search/replace is actually processing it twice. (I would recommend maybe using the system-type variable instead to determine whether to do that slash replacement at all and determine the search value up front to avoid the second processing).

Instead - I deleted that jdee-flycheck--restore-original-filename method entirely and modified jdee-flycheck-compile-buffer-after to not make that call.

(defun jdee-flycheck-compile-buffer-after (checker cback orig-file orig-buffer
                                                   temp-file temp-buffer)
  "Callback function called when the compilation is complete.
Looks for errors and converts then to flycheck errors.  Also
cleans up after the compilation."
  (lambda (buf msg)
    (with-current-buffer buf
      ;;(jdee-flycheck--restore-original-filename temp-file orig-file)
      (let ((errors (jdee-flycheck--collect-errors checker orig-file temp-file orig-buffer)))
        (flycheck-display-error-messages errors)

Note - I also modified the call to the jdee-flycheck--collect-errors method to add 2 new arguments to (orig-file and temp-file)

Then I modified jdee-flycheck--collect-errors to handle the pathname properly by relying on file-name-directory and file-name-nondirectory (which are platform agnostic - they produce the same output regardless of whether the input path was unix or windows path styling) [unfortunately there doesn't seem to be a "pathname" method that does the same on a full path so I had to break it up using the two methods]

(defun jdee-flycheck--collect-errors (checker orig-file temp-file orig-buffer)
  "Collect flycheck errors from `CHECKER' run `ORIG-BUFFER'."
  (goto-char (point-min))
  (let ((errors nil)
	(temp-dir (file-name-directory temp-file))  ;; platform agnostic path directory
	(temp-filename (file-name-nondirectory temp-file)))   ;; platform agnostic filename
    (while (jdee-flycheck-find-next-error)
      (forward-line 2)
      (let ((file (match-string 1)) ;; file at this point is the temp directory path
            (line (match-string 2))
            (message (match-string 3))
            ;; jdee-flymake-get-col changes search data; do it last
            (col (jdee-flymake-get-col)))
	;; If the error list from the compiler includes a file from the classpath
	;;  that wasn't the current buffer, we don't want to create a flycheck error
	;;  with a buffer name and filename that don't actually correspond.
	;; Filter here to only create the flycheck error if it's for the current buffer
	(when (and (equal temp-dir (file-name-directory file))
		   (equal temp-filename (file-name-nondirectory file)))
	  (cl-pushnew
	   (jdee-flycheck-compile-buffer-error checker
					       orig-file ;; replace with orig-file
					       line
					       col
					       message
					       orig-buffer)
	   errors))))
    errors))

This solution does leave me with the theoretical issue that if the pathname of the file for some reason occurred in the actual message body of the error message then it would still be referring to the temp path. But I don't actually think that ever happens (please let me know if you are aware of any conditions under which this might occur). [If it does - it would be still quicker to do the regex replacement on the message just before it goes into the constructor]

This drops the first regex search/replace entirely on the temp buffer and also addresses another odd issue where compiler errors for other files would end up being parsed out and associated with the current buffer (is that intentional?)

I have been unable to find any reason why having the buffer updated is needed beyond this parsing issue (in fact a couple lines later that buffer is killed - so I think it's safe)

Additionally - a slight tangent but related in the method jdee-flycheck-compile-buffer I also made the temp buffer read only (because it was getting annoying having emacs tell me the file on disk is read only and did I want to also make the buffer read only. I did this after populating the buffer.

(defun jdee-flycheck-compile-buffer (checker cback &optional buffer)
...
    (with-current-buffer (generate-new-buffer name)
      (make-local-variable 'jdee-flycheck-temp-files)
      (cl-pushnew dir jdee-flycheck-temp-files)
      (insert-file-contents-literally temp-file)
      (setq buffer-file-name temp-file)
      (read-only-mode)  ;; make temp buffer read-only

      (add-hook 'kill-buffer-hook 'jdee-flycheck-cleanup nil t)
...

Do you perhaps know if there's a different root cause for this?

nicholsonscn avatar Jul 05 '17 18:07 nicholsonscn

Hi,,

I seem to be running into a similar problem as I get an "error in process filter: removing directory: permission denied, c:/Users/Sahil/AppData/Local/Temp/JDEE_flycheck_zxfiG9". I've tried a few of the methods that you have suggested but it doesn't seem to be working.

I'm fairly new to emacs, so I haven't really worked to much with customization. However, just the setup steps seem to start this problem.

Any help would be amazing! Thank you in advance!

sahiljain11 avatar Sep 02 '19 22:09 sahiljain11