Port to linter-plus

[daw42]

This pull request ports linter javac to the new Linter API.

It also uses a longer regular expression to extract the column number for errors and warnings.

This pull request relates to issue #20.

[keplersj]

This should be changed to javac which will auto-resolve to /usr/bin/javac in 90% of cases. In the other 10% it will resolve to something else.

[keplersj]

Besides that, it looks good to me. It looks like you should be able to expand your Regex to find the file being affected. I've written some code which should work here for another linter: https://github.com/AtomLinter/linter-swiftc/blob/master/lib/provider.coffee#L39

Anything I've missed @steelbrain?

[steelbrain]

@k2b6s9j the spawning part could very much use the helpers module.

[steelbrain]

Also @daw42, does javac support JSON output? It should be a priority to use JSON instead of regex if CLI linter supports it.
Also lintOnFly means that the file will be linted everytime you'll stop writing and I don't see you writing the rest of textEditor.getText() to the stdin of the spawned process, which mean that it'll lint the file on the filesystem everytime.
You can either use the helpers to easily spawn the process and write input to stdin instead of a file, or you can disable lintOnFly.

[keplersj]

javac definitely doesn't have a JSON output. Java moves very slowly and was created 20 years ago.

[steelbrain]

😆

[daw42]

@steelbrain Good point about the lintOnFly option. I think that I'll just disable it and always lint the file on the filesystem. Later on, it might be good to make this a configuration option and give the user the choice.

[daw42]

Just updated the pull request branch. Thanks for your feedback!

By the way, looks like javac doesn't support compiling from standard input, so lintOnFly needs to be disabled.

[steelbrain]

@daw42 I am so sorry this took so much time. You should've pinged us.
Anyways, can you use the atom-linter npm module for spawning part please?

[daw42]

@steelbrain Ok, I've updated so that it uses atom-linter for spawning javac. Thanks.

[steelbrain]

You don't actually have to catch it here, the last time I checked, atom-linter throws this and linter catches

[daw42]

Updated, thanks again. :)

[steelbrain]

Beautiful, I'll test it soon.

[steelbrain]

I just tried it and it didn't work for me, here's the javacOutput

javac: invalid flag: /var/web/PublishSpace/Zoom/API/test.Java
Usage: javac <options> <source files>
use -help for a list of possible options

[steelbrain]

Here's the patch that fixes that issue

diff --git a/.eslintrc b/.eslintrc
diff --git a/lib/init.coffee b/lib/init.coffee
index e2432ad..2d3bd64 100755
--- a/lib/init.coffee
+++ b/lib/init.coffee
@@ -24,12 +24,10 @@ module.exports =
     lintOnFly: false       # Only lint on save
     lint: (textEditor) =>
       filePath = textEditor.getPath()
-      wd = path.dirname filePath
       # Use the text editor's working directory as the classpath.
       #  TODO: Make the classpath user configurable.
-      args = ['-Xlint:all', '-cp', wd, filePath]
-      messages = []
-      helpers.exec(@javaExecutablePath, args, {stream: 'stderr'})
+      args = ['-Xlint:all', '-cp', '.', path.basename(filePath)]
+      helpers.exec(@javaExecutablePath, args, {stream: 'stderr', cwd: path.dirname(filePath)})
         .then (val) => return @parse(val, textEditor)

   parse: (javacOutput, textEditor) =>

Also, Your current regex is failing for me as the javacOutput is

error: Class names, 'test.Java', are only accepted if annotation processing is explicitly requested
1 error

[daw42]

Looks like the problem is that javac expects a lower-case 'j' for the file extension. Without that, it seems to think that it is a flag or a class name (depending on the classpath). For example, I just tried this in the working directory:

$ javac test.Java 
error: Class names, 'test.Java', are only accepted if annotation processing is explicitly requested
1 error

$ javac ./test.Java 
javac: invalid flag: ./test.Java
Usage: javac <options> <source files>
use -help for a list of possible options

Try a file that has an extension that's all lower case.

I like the patch. Better to base everything from the working directory.

The regex targets only file syntax type errors, I might need to expand it to handle errors like the one you found.

[daw42]

The cwd option for exec doesn't work for me. The source code of atom-linter doesn't seem to use it.

[keplersj]

cwd is passed directly to child_process. I'll double check later, but
that's how I wrote it initially.

On Fri, Jul 31, 2015, 3:38 PM daw42 [email protected] wrote:

The cwd option for exec doesn't work for me. The source code of
atom-linter doesn't seem to use it.

—
Reply to this email directly or view it on GitHub
#23 (comment)
.

[steelbrain]

@k2b6s9j You're correct! All of the options, not just cwd are passed directly, some are however interpreted by atom-linter and others by the node core

[daw42]

@k2b6s9j @steelbrain

helpers.exec('pwd', [])
  .then (val) => console.log("Current dir: #{val}")

helpers.exec('pwd', [], {cwd: '/tmp'} )
  .then (val) => console.log("Current dir (cwd): #{val}")

Output:

Current dir (cwd): /
Current dir: /

[steelbrain]

Weird, I'll investigate when I have some time. That is definitely not the expected behavior.

[keplersj]

Sounds like we need a new unit test too. Mind filing an issue on the repo
so I know to write the regression test when I get home?

On Sat, Aug 1, 2015, 10:59 AM Steel Brain [email protected] wrote:

Weird, I'll investigate when I have some time. That is definitely not the
expected behavior.

—
Reply to this email directly or view it on GitHub
#23 (comment)
.

[daw42]

javac will not accept a file with .Java extension. Please test with *.java.

[keplersj]

Instead of (error|warning): I recommend using (.+):. This does not limit the type of issue to error and warning. Future-proofing incase some new type of issue is introduced in the future.

[keplersj]

Just tested, and it looks good. I'll merge and make that change to the regex. Thanks @daw42!