diff --git a/README.md b/README.md index 4cc4bed..8a8def9 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,7 @@ Example configuration: jslint target/jshint.xml + false false @@ -72,14 +73,15 @@ Configuration options: | Option | Default value | Explanation | | ---------------: | :---------------------------: | -------------| -| version | 2.4.1 | Selects which embedded version of jshint will be used | +| version | 2.4.3 | Selects which embedded version of jshint will be used | | options | | List of comma-separated [JSHint options](http://www.jshint.com/docs/#options) | | globals | | List of comma-separated [JSHint globals](http://www.jshint.com/docs/#usage) | -| configFile | | Path to a JSHint JSON config file. Its contents will override values set in `options` and `globals`, if present. Please note that block and line comments will be stripped prior to processing so it's OK to include them. | +| configFile | | Path to a JSHint JSON config file. Its contents will override values set in `options` and `globals`, if present. Can be relative or absolute file path. If not set or found, a search from project base up to the file system root will be made, looking for a file named '.jshintrc'. If none found, only the configured `globals` will be used for configuration. Please note that block and line comments will be stripped prior to processing so it's OK to include them. | | directories | `src` | Locations in which the plugin will search for *.js files | | excludes | | Excludes are resolved relative to the basedir of the module | | reporter | | If present, JSHint will generate a reporting file which can be used for some CI tools. Currently, only `jslint` and `checkstyle` format are supported. | | reportFile | target/jshint.xml | Path to an output reporting file | | failOnError | true | Controls whether the plugin fails the build when JSHint is unhappy. Setting this to `false` is discouraged, as it removes most of the benefit of using this plugin. Instead, if you have problem files that you can't fix [disable/override JSHint on a per-file basis](http://www.jshint.com/docs/#config), or tell the plugin to specifically exclude them in the `excludes` section | +| failOnWarning | true | Same as `failOnError`, but with warnings. Setting this to true is recommended and the default behavior as in jshint-mojo <=v1.3.0. | diff --git a/pom.xml b/pom.xml index 0c72459..1362834 100644 --- a/pom.xml +++ b/pom.xml @@ -12,26 +12,25 @@ 1.3.1-SNAPSHOT maven-plugin - ${project.artifactId} + ${project.artifactId} - - - - org.apache.maven.plugins - maven-compiler-plugin - 2.3.2 - - 1.6 - 1.6 - - - - org.apache.maven.plugins - maven-release-plugin - 2.4 - - + + + org.apache.maven.plugins + maven-compiler-plugin + 2.3.2 + + 1.6 + 1.6 + + + + org.apache.maven.plugins + maven-release-plugin + 2.4 + + @@ -66,35 +65,39 @@ 4.11 test - - commons-lang - commons-lang - 2.6 - + + commons-lang + commons-lang + 2.6 + a maven mojo that integrates the 'jshint' javascript preprocessor - https://github.com/cjdev/httpobjects - - - GPL 2 + classpath exception - https://raw.github.com/cjdev/jshint-mojo/master/LICENSE - - + https://github.com/cjdev/httpobjects + + + GPL 2 + classpath exception + https://raw.github.com/cjdev/jshint-mojo/master/LICENSE + + - scm:git:git@github.com:cjdev/jshint-mojo.git - scm:git:git@github.com:cjdev/jshint-mojo.git - scm:git:git@github.com:cjdev/jshint-mojo.git - HEAD - + scm:git:git@github.com:cjdev/jshint-mojo.git + scm:git:git@github.com:cjdev/jshint-mojo.git + scm:git:git@github.com:cjdev/jshint-mojo.git + HEAD + - - - stu - Stu Penrose - stu@penrose.us - + + + stu + Stu Penrose + stu@penrose.us + + + Sebl29 + Sebl29@github.com + - + diff --git a/src/main/java/com/cj/jshintmojo/DumpCacheMojo.java b/src/main/java/com/cj/jshintmojo/DumpCacheMojo.java index 37f6b83..cb24566 100644 --- a/src/main/java/com/cj/jshintmojo/DumpCacheMojo.java +++ b/src/main/java/com/cj/jshintmojo/DumpCacheMojo.java @@ -22,24 +22,25 @@ public class DumpCacheMojo extends AbstractMojo { * @required */ private File basedir; - - public void execute() throws MojoExecutionException, MojoFailureException { + + @Override + public void execute() throws MojoExecutionException, MojoFailureException { try { - + final File cachePath = new File(basedir, "target/lint.cache"); - + final Map entries; if(cachePath.exists()){ entries = Util.readObject(cachePath); }else{ entries = new HashMap(); } - + for(Result r : entries.values()){ - String status = r.errors.isEmpty()?"[OK]":"[BAD]"; + String status = r.hints.isEmpty()?"[OK]":"[BAD]"; System.out.println(status + " " + r.path); } - + } catch (Exception e) { throw new MojoExecutionException("Something bad happened", e); } diff --git a/src/main/java/com/cj/jshintmojo/Mojo.java b/src/main/java/com/cj/jshintmojo/Mojo.java index e32546b..bd95708 100644 --- a/src/main/java/com/cj/jshintmojo/Mojo.java +++ b/src/main/java/com/cj/jshintmojo/Mojo.java @@ -1,6 +1,6 @@ package com.cj.jshintmojo; -import static com.cj.jshintmojo.util.Util.*; +import static com.cj.jshintmojo.util.Util.mkdirs; import java.io.BufferedWriter; import java.io.File; @@ -11,6 +11,7 @@ import java.io.OutputStreamWriter; import java.io.Writer; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -30,13 +31,12 @@ import com.cj.jshintmojo.jshint.FunctionalJava; import com.cj.jshintmojo.jshint.FunctionalJava.Fn; import com.cj.jshintmojo.jshint.JSHint; -import com.cj.jshintmojo.jshint.JSHint.Error; +import com.cj.jshintmojo.jshint.JSHint.Hint; import com.cj.jshintmojo.reporter.CheckStyleReporter; import com.cj.jshintmojo.reporter.JSHintReporter; import com.cj.jshintmojo.reporter.JSLintReporter; import com.cj.jshintmojo.util.OptionsParser; import com.cj.jshintmojo.util.Util; -import java.util.Collections; /** * @goal lint @@ -61,7 +61,7 @@ public class Mojo extends AbstractMojo { private String options = null; /** - * @parameter + * @parameter */ private String globals = ""; @@ -88,12 +88,16 @@ public class Mojo extends AbstractMojo { /** * @parameter expression="${jshint.version}" */ - private String version = "2.4.3"; - + private final String version = "2.4.3"; + /** - * @parameter + * @parameter */ private Boolean failOnError = true; + /** + * @parameter + */ + private Boolean failOnWarning = true; /** * @parameter default-value="${basedir} @@ -101,28 +105,32 @@ public class Mojo extends AbstractMojo { * @required */ File basedir; - + public Mojo() {} - - public Mojo(String options, String globals, File basedir, List directories, List excludes, boolean failOnError, String configFile, String reporter, String reportFile, String ignoreFile) { + + public Mojo(final String options, final String globals, final File basedir, final List directories, + final List excludes, final boolean failOnError, final boolean failOnWarning, + final String configFile, final String reporter, final String reportFile, final String ignoreFile) { super(); this.options = options; this.globals = globals; this.basedir = basedir; this.directories.addAll(directories); this.excludes.addAll(excludes); - this.failOnError = failOnError; + this.failOnError = failOnError; + this.failOnWarning = failOnWarning; this.configFile = configFile; this.reporter = reporter; this.reportFile = reportFile; this.ignoreFile = ignoreFile; } - - public void execute() throws MojoExecutionException, MojoFailureException { + + @Override + public void execute() throws MojoExecutionException, MojoFailureException { getLog().info("using jshint version " + version); final String jshintCode = getEmbeddedJshintCode(version); - + final JSHint jshint = new JSHint(jshintCode); final Config config = readConfig(this.options, this.globals, this.configFile, this.basedir, getLog()); @@ -130,60 +138,67 @@ public void execute() throws MojoExecutionException, MojoFailureException { this.excludes.addAll(readIgnore(this.ignoreFile, this.basedir, getLog()).lines); } final Cache.Hash cacheHash = new Cache.Hash(config.options, config.globals, this.version, this.configFile, this.directories, this.excludes); - + if(directories.isEmpty()){ directories.add("src"); } - + try { final File targetPath = new File(basedir, "target"); mkdirs(targetPath); final File cachePath = new File(targetPath, "lint.cache"); final Cache cache = readCache(cachePath, cacheHash); - + final List files = findFilesToCheck(); final Map currentResults = lintTheFiles(jshint, cache, files, config, getLog()); - + Util.writeObject(new Cache(cacheHash, currentResults), cachePath); - + handleResults(currentResults, this.reporter, this.reportFile); - + } catch (FileNotFoundException e) { throw new MojoExecutionException("Something bad happened", e); } } - + static class Config { final String options, globals; - public Config(String options, String globals) { + public Config(final String options, final String globals) { super(); this.options = options; this.globals = globals; } - + } - - private static Config readConfig(String options, String globals, String configFileParam, File basedir, Log log) throws MojoExecutionException { - final File jshintRc = findJshintrc(basedir); - final File configFile = StringUtils.isNotBlank(configFileParam)?new File(basedir, configFileParam):null; - + + private static Config readConfig(final String options, final String globals, final String configFileParam, final File basedir, final Log log) throws MojoExecutionException { final Config config; - if(options==null){ - if(configFile!=null){ - log.info("Using configuration file: " + configFile.getAbsolutePath()); + if (options != null) { + config = new Config(options, globals); + } else { + File configFile = null; + if (StringUtils.isNotBlank(configFileParam)) { + configFile = new File(configFileParam); + if (!configFile.isAbsolute()) { + configFile = new File(basedir, configFileParam); + } + } + if (configFile != null && configFile.exists()) { + log.info("Using configured 'configFile' from: " + configFile.getAbsolutePath()); config = processConfigFile(configFile); - }else if(jshintRc!=null){ - log.info("Using configuration file: " + jshintRc.getAbsolutePath()); - config = processConfigFile(jshintRc); - }else{ - config = new Config("", globals); + } else { + final File jshintRc = findJshintrc(basedir); + if (jshintRc != null) { + log.info("No configFile configured or found, but found a '.jshintrc' file: " + jshintRc.getAbsolutePath()); + config = processConfigFile(jshintRc); + } else { + log.info("No options, configFile or '.jshintrc' file found. Only using configured globals."); + config = new Config("", globals); + } } - }else{ - config = new Config(options, globals); } - return config; } @@ -191,13 +206,13 @@ static class Ignore { final List lines; - public Ignore(List lines) { + public Ignore(final List lines) { this.lines = lines; } } - private static Ignore readIgnore(String ignoreFileParam, File basedir, Log log) throws MojoExecutionException { + private static Ignore readIgnore(final String ignoreFileParam, final File basedir, final Log log) throws MojoExecutionException { final File jshintignore = findJshintignore(basedir); final File ignoreFile = StringUtils.isNotBlank(ignoreFileParam) ? new File(basedir, ignoreFileParam) : null; @@ -221,18 +236,19 @@ private List findFilesToCheck() { for(String next: directories){ File path = new File(basedir, next); if(!path.exists() && !path.isDirectory()){ - getLog().warn("You told me to find tests in " + next + ", but there is nothing there (" + path.getAbsolutePath() + ")"); + getLog().info("You told me to find tests in " + next + ", but there is nothing there (" + path.getAbsolutePath() + ")"); }else{ collect(path, javascriptFiles); } } List matches = FunctionalJava.filter(javascriptFiles, new Fn(){ - public Boolean apply(File i) { + @Override + public Boolean apply(final File i) { for(String exclude : excludes){ File e = new File(basedir, exclude); if(i.getAbsolutePath().startsWith(e.getAbsolutePath())){ - getLog().warn("Excluding " + i); + getLog().info("Excluding " + i); return Boolean.FALSE; } } @@ -243,82 +259,115 @@ public Boolean apply(File i) { return matches; } - private static Map lintTheFiles(final JSHint jshint, final Cache cache, List filesToCheck, final Config config, final Log log) throws FileNotFoundException { + private static Map lintTheFiles(final JSHint jshint, final Cache cache, final List filesToCheck, final Config config, final Log log) throws FileNotFoundException { final Map currentResults = new HashMap(); - for(File file : filesToCheck){ - Result previousResult = cache.previousResults.get(file.getAbsolutePath()); - Result theResult; - if(previousResult==null || (previousResult.lastModified.longValue()!=file.lastModified())){ - log.info(" " + file ); - List errors = jshint.run(new FileInputStream(file), config.options, config.globals); - theResult = new Result(file.getAbsolutePath(), file.lastModified(), errors); - }else{ - log.info(" " + file + " [no change]"); - theResult = previousResult; - } - - if(theResult!=null){ - currentResults.put(theResult.path, theResult); - Result r = theResult; - currentResults.put(r.path, r); - for(Error error: r.errors){ - log.error(" " + error.line.intValue() + "," + error.character.intValue() + ": " + error.reason); - } - } + for (File file : filesToCheck) { + Result previousResult = cache.previousResults.get(file.getAbsolutePath()); + Result theResult; + if (previousResult == null || (previousResult.lastModified.longValue() != file.lastModified())) { + log.info(" " + file); + List hints = jshint.run(new FileInputStream(file), config.options, config.globals); + theResult = new Result(file.getAbsolutePath(), file.lastModified(), hints); + } else { + log.info(" " + file + " [no change]"); + theResult = previousResult; + } + + if (theResult != null) { + currentResults.put(theResult.path, theResult); + Result r = theResult; + currentResults.put(r.path, r); + for (Hint hint : r.hints) { + String consoleLogMessage = hint.printLogMessage(); + if (hint instanceof JSHint.Info) { + log.info(consoleLogMessage); + } else if (hint instanceof JSHint.Warning) { + log.warn(consoleLogMessage); + } else if (hint instanceof JSHint.Error) { + log.error(consoleLogMessage); + } + } + } } return currentResults; } private void handleResults(final Map currentResults, - final String reporter, final String reportFile) throws MojoExecutionException - { + final String reporter, final String reportFile) throws MojoExecutionException { char NEWLINE = '\n'; StringBuilder errorRecap = new StringBuilder(NEWLINE); - + int numProblematicFiles = 0; + boolean hasErrors = false; + boolean hasWarnings = false; for(Result r : currentResults.values()){ - if(!r.errors.isEmpty()){ - numProblematicFiles ++; + if (!r.hints.isEmpty()) { + numProblematicFiles ++; errorRecap .append(NEWLINE) .append(r.path) .append(NEWLINE); - for(Error error: r.errors){ - errorRecap - .append(" ") - .append(error.line.intValue()) - .append(",") - .append(error.character.intValue()) - .append(": ") - .append(error.reason) + for (Hint hint : r.hints) { + if (hint == null) { + errorRecap.append("!!!hint was null!"); + continue; + } + hasWarnings |= (hint instanceof JSHint.Warning); + hasErrors |= (hint instanceof JSHint.Error); + + errorRecap + .append(hint.printLogMessage()) .append(NEWLINE); } } } - + if(numProblematicFiles > 0) { saveReportFile(currentResults, reporter, reportFile); - String errorMessage = "\nJSHint found problems with " + numProblematicFiles + " file"; - - // pluralise - if (numProblematicFiles > 1) { - errorMessage += "s"; - } - - errorMessage += errorRecap.toString(); + String errorMessage = "\nJSHint found "; + + if (hasErrors) + errorMessage += "errors "; + if (hasErrors && hasWarnings) + errorMessage += "and "; + if (hasWarnings) + errorMessage += "warnings "; + + errorMessage += "in " + numProblematicFiles + " file"; + // pluralise + if (numProblematicFiles > 1) { + errorMessage += "s"; + } + errorMessage += "! Please see errors/warning above!"; + - if (failOnError) { - throw new MojoExecutionException(errorMessage); - } else { - getLog().info(errorMessage); - } + errorMessage += "\nJSHint is "; + if (!failOnError && !failOnWarning) { + errorMessage += "not configured to fail on error or warning."; + } else { + errorMessage += "configured to fail on "; + if (failOnError) + errorMessage += "error "; + if (failOnError && failOnWarning) + errorMessage += "or "; + if (failOnWarning) + errorMessage += "warning "; + } + + if (hasErrors || hasWarnings) + + if ((failOnError && hasErrors) || (failOnWarning && hasWarnings)) { + throw new MojoExecutionException(errorMessage); + } else { + getLog().info(errorMessage); + } } } - private void saveReportFile(Map results, String reportType, String reportFile) { + private void saveReportFile(final Map results, final String reportType, final String reportFile) { JSHintReporter reporter = null; if(JSLintReporter.FORMAT.equalsIgnoreCase(reportType)){ reporter = new JSLintReporter(); @@ -354,8 +403,8 @@ private void saveReportFile(Map results, String reportType, Stri } @SuppressWarnings("serial") - private static String getEmbeddedJshintCode(String version) throws MojoFailureException { - + private static String getEmbeddedJshintCode(final String version) throws MojoFailureException { + final String resource = EmbeddedJshintCode.EMBEDDED_VERSIONS.get(version); if(resource==null){ StringBuffer knownVersions = new StringBuffer(); @@ -366,8 +415,8 @@ private static String getEmbeddedJshintCode(String version) throws MojoFailureEx } return resource; } - - private static File findJshintrc(File cwd) { + + private static File findJshintrc(final File cwd) { File placeToLook = cwd; while(placeToLook.getParentFile()!=null){ File rcFile = new File(placeToLook, ".jshintrc"); @@ -377,11 +426,11 @@ private static File findJshintrc(File cwd) { placeToLook = placeToLook.getParentFile(); } } - + return null; } - private static File findJshintignore(File cwd) { + private static File findJshintignore(final File cwd) { File placeToLook = cwd; while (placeToLook.getParentFile() != null) { File ignoreFile = new File(placeToLook, ".jshintignore"); @@ -395,13 +444,17 @@ private static File findJshintignore(File cwd) { return null; } - private static boolean nullSafeEquals(String a, String b) { - if(a==null && b==null) return true; - else if(a==null || b==null) return false; - else return a.equals(b); + private static boolean nullSafeEquals(final String a, final String b) { + if(a==null && b==null) { + return true; + } else if(a==null || b==null) { + return false; + } else { + return a.equals(b); + } } - private Cache readCache(File path, Cache.Hash hash){ + private Cache readCache(final File path, final Cache.Hash hash){ try { if(path.exists()){ Cache cache = Util.readObject(path); @@ -411,16 +464,16 @@ private Cache readCache(File path, Cache.Hash hash){ getLog().warn("Something changed ... clearing cache"); return new Cache(hash); } - + } } catch (Throwable e) { - super.getLog().warn("I was unable to read the cache. This may be because of an upgrade to the plugin."); + super.getLog().warn("I was unable to read the cache. This may be because of an upgrade to the plugin."); } - + return new Cache(hash); } - - private void collect(File directory, List files) { + + private void collect(final File directory, final List files) { for(File next : directory.listFiles()){ if(next.isDirectory()){ collect(next, files); @@ -435,7 +488,7 @@ private void collect(File directory, List files) { * * @throws MojoExecutionException if the specified file cannot be processed */ - private static Config processConfigFile(File configFile) throws MojoExecutionException { + private static Config processConfigFile(final File configFile) throws MojoExecutionException { byte[] configFileContents; try { configFileContents = FileUtils.readFileToByteArray(configFile); @@ -447,7 +500,7 @@ private static Config processConfigFile(File configFile) throws MojoExecutionExc Set optionsSet = OptionsParser.extractOptions(configFileContents); final String globals, options; - + if (globalsSet.size() > 0) { globals = StringUtils.join(globalsSet.iterator(), ","); }else{ @@ -459,7 +512,7 @@ private static Config processConfigFile(File configFile) throws MojoExecutionExc }else{ options = ""; } - + return new Config(options, globals); } @@ -469,7 +522,7 @@ private static Config processConfigFile(File configFile) throws MojoExecutionExc * * @throws MojoExecutionException if the specified file cannot be processed */ - private static Ignore processIgnoreFile(File ignoreFile) throws MojoExecutionException { + private static Ignore processIgnoreFile(final File ignoreFile) throws MojoExecutionException { try { return new Ignore(FileUtils.readLines(ignoreFile, "UTF-8")); } catch (IOException e) { diff --git a/src/main/java/com/cj/jshintmojo/cache/Result.java b/src/main/java/com/cj/jshintmojo/cache/Result.java index e87ee95..7d1b3e1 100644 --- a/src/main/java/com/cj/jshintmojo/cache/Result.java +++ b/src/main/java/com/cj/jshintmojo/cache/Result.java @@ -3,18 +3,18 @@ import java.io.Serializable; import java.util.List; -import com.cj.jshintmojo.jshint.JSHint.Error; +import com.cj.jshintmojo.jshint.JSHint.Hint; @SuppressWarnings("serial") public class Result implements Serializable { public final String path; public final Long lastModified; - public final List errors; - - public Result(String path, Long lastModified, List errors) { + public final List hints; + + public Result(final String path, final Long lastModified, final List hints) { super(); this.path = path; this.lastModified = lastModified; - this.errors = errors; + this.hints = hints; } } \ No newline at end of file diff --git a/src/main/java/com/cj/jshintmojo/jshint/JSHint.java b/src/main/java/com/cj/jshintmojo/jshint/JSHint.java index 23dd4ae..6b6b946 100644 --- a/src/main/java/com/cj/jshintmojo/jshint/JSHint.java +++ b/src/main/java/com/cj/jshintmojo/jshint/JSHint.java @@ -9,35 +9,36 @@ import org.mozilla.javascript.EcmaError; import org.mozilla.javascript.NativeArray; import org.mozilla.javascript.NativeObject; +import org.mozilla.javascript.ScriptableObject; import com.cj.jshintmojo.util.Rhino; public class JSHint { - + private final Rhino rhino; - - public JSHint(String jshintCode) { - + + public JSHint(final String jshintCode) { + rhino = new Rhino(); try { rhino.eval( "print=function(){};" + "quit=function(){};" + "arguments=[];"); - + rhino.eval(commentOutTheShebang(resourceAsString(jshintCode))); } catch (EcmaError e) { throw new RuntimeException("Javascript eval error:" + e.getScriptStackTrace(), e); } } - private String commentOutTheShebang(String code) { + private String commentOutTheShebang(final String code) { String minusShebang = code.startsWith("#!")?"//" + code : code; return minusShebang; } - public List run(InputStream source, String options, String globals) { - final List results = new ArrayList(); + public List run(final InputStream source, final String options, final String globals) { + final List results = new ArrayList(); String sourceAsText = toString(source); @@ -50,9 +51,11 @@ public List run(InputStream source, String options, String globals) { NativeArray errors = rhino.eval("JSHINT.errors"); for(Object next : errors){ - if(next!=null){ // sometimes it seems that the last error in the list is null - Error error = new Error(new JSObject(next)); - results.add(error); + if(next != null){ // sometimes it seems that the last error in the list is null + JSObject jso = new JSObject(next); + if (jso.dot("id").toString().equals("(error)")) { + results.add(Hint.createHint(jso)); + } } } } @@ -60,7 +63,7 @@ public List run(InputStream source, String options, String globals) { return results; } - private NativeObject toJsObject(String options) { + private NativeObject toJsObject(final String options) { NativeObject nativeOptions = new NativeObject(); for (final String nextOption : options.split(",")) { final String option = nextOption.trim(); @@ -82,16 +85,16 @@ private NativeObject toJsObject(String options) { } else if (rest.equals("false")) { value = Boolean.FALSE; } else { - value = rest; + value = rest; } } - nativeOptions.defineProperty(name, value, NativeObject.READONLY); + nativeOptions.defineProperty(name, value, ScriptableObject.READONLY); } } return nativeOptions; } - private static String toString(InputStream in) { + private static String toString(final InputStream in) { try { return IOUtils.toString(in); } catch (Exception e) { @@ -99,30 +102,31 @@ private static String toString(InputStream in) { } } - private String resourceAsString(String name) { + private String resourceAsString(final String name) { return toString(getClass().getResourceAsStream(name)); } - @SuppressWarnings("unchecked") + @SuppressWarnings("unchecked") static class JSObject { - private NativeObject a; + private final NativeObject a; - public JSObject(Object o) { - if(o==null) throw new NullPointerException(); + public JSObject(final Object o) { + if(o==null) { + throw new NullPointerException(); + } this.a = (NativeObject)o; } - public T dot(String name){ + public T dot(final String name){ return (T) a.get(name); } } - @SuppressWarnings("serial") - public static class Error implements Serializable { + public static abstract class Hint { public String id, code, raw, evidence, reason; public Number line, character; - public Error(JSObject o) { + public Hint(final JSObject o) { id = o.dot("id"); code = o.dot("code"); raw = o.dot("raw"); @@ -132,8 +136,69 @@ public Error(JSObject o) { reason = o.dot("reason"); } + public Hint() { } + + public String printLogMessage() { + String line = (this.line != null) ? String.valueOf(this.line.intValue()) : ""; + String character = (this.character != null) ? String.valueOf(this.character.intValue()) : ""; + return " " + line + "," + character + ": " + this.reason + " \t(" + this.code + ")"; + } + + public static Hint createHint(final JSObject jso) { + if (jso == null) + throw new IllegalArgumentException(); + String code = (String)jso.dot("code"); + + char c = code.charAt(0); + Hint hint; + switch (c) { + case 'E': + hint = new Error(jso); + break; + case 'W': + hint = new Warning(jso); + break; + case 'I': + hint = new Info(jso); + break; + default: + throw new IllegalArgumentException("Unexpected char c=" + c); + } + return hint; + } + + } + + @SuppressWarnings("serial") + public static class Warning extends Hint implements Serializable { + + public Warning(final JSObject o) { + super(o); + } + + // NOTE: for Unit Testing purpose. + public Warning() { } + } + + @SuppressWarnings("serial") + public static class Error extends Hint implements Serializable { + + public Error(final JSObject o) { + super(o); + } + // NOTE: for Unit Testing purpose. - public Error() { + public Error() { } + } + + @SuppressWarnings("serial") + public static class Info extends Hint implements Serializable { + + public Info(final JSObject o) { + super(o); } + + // NOTE: for Unit Testing purpose. + public Info() { } } } diff --git a/src/main/java/com/cj/jshintmojo/reporter/CheckStyleReporter.java b/src/main/java/com/cj/jshintmojo/reporter/CheckStyleReporter.java index e563a37..28fb56c 100644 --- a/src/main/java/com/cj/jshintmojo/reporter/CheckStyleReporter.java +++ b/src/main/java/com/cj/jshintmojo/reporter/CheckStyleReporter.java @@ -6,7 +6,7 @@ import org.apache.commons.lang.StringUtils; import com.cj.jshintmojo.cache.Result; -import com.cj.jshintmojo.jshint.JSHint; +import com.cj.jshintmojo.jshint.JSHint.Hint; /** * CheckStyle style xml reporter class. @@ -19,7 +19,7 @@ public class CheckStyleReporter implements JSHintReporter { public static final String FORMAT = "checkstyle"; @Override - public String report(Map results) { + public String report(final Map results) { if(results == null){ return ""; } @@ -31,9 +31,9 @@ public String report(Map results) { for(String file : files){ Result result = results.get(file); buf.append("\t\n"); - for(JSHint.Error error : result.errors){ - buf.append(String.format("\t\t\n", - error.line.intValue(), error.character.intValue(), encode(error.reason), encode(error.code), severity(error.code))); + for (Hint hint : result.hints) { + buf.append(String.format("\t\t<" + severity(hint.code) + " line=\"%d\" column=\"%d\" message=\"%s\" source=\"jshint.%s\" severity=\"%s\" />\n", + hint.line.intValue(), hint.character.intValue(), encode(hint.reason), encode(hint.code), severity(hint.code))); } buf.append("\t\n"); } @@ -41,23 +41,22 @@ public String report(Map results) { return buf.toString(); } - - private String severity(String errorCode) { + + private String severity(final String errorCode) { if(StringUtils.isNotEmpty(errorCode)){ switch(errorCode.charAt(0)){ case 'E': return "error"; + case 'W': + return "warning"; case 'I': return "info"; - case 'W': - default: - break; } } - return "warning"; + throw new IllegalArgumentException(); } - - private String encode(String str) { + + private String encode(final String str) { if(str == null){ return ""; } diff --git a/src/main/java/com/cj/jshintmojo/reporter/JSLintReporter.java b/src/main/java/com/cj/jshintmojo/reporter/JSLintReporter.java index fe6d45c..4d9c15f 100644 --- a/src/main/java/com/cj/jshintmojo/reporter/JSLintReporter.java +++ b/src/main/java/com/cj/jshintmojo/reporter/JSLintReporter.java @@ -6,7 +6,7 @@ import org.apache.commons.lang.StringUtils; import com.cj.jshintmojo.cache.Result; -import com.cj.jshintmojo.jshint.JSHint; +import com.cj.jshintmojo.jshint.JSHint.Hint; /** * JSLint style xml reporter class. @@ -19,7 +19,7 @@ public class JSLintReporter implements JSHintReporter { public static final String FORMAT = "jslint"; @Override - public String report(Map results) { + public String report(final Map results) { if(results == null){ return ""; } @@ -31,11 +31,11 @@ public String report(Map results) { for(String file : files){ Result result = results.get(file); buf.append("\t\n"); - for(JSHint.Error issue : result.errors){ + for (Hint hint : result.hints) { buf.append(String.format("\t\t\n"); } @@ -45,8 +45,8 @@ public String report(Map results) { return buf.toString(); } - - private String encode(String str) { + + private String encode(final String str) { if(str == null){ return ""; } diff --git a/src/test/java/com/cj/jshintmojo/EmbeddedVersionsTest.java b/src/test/java/com/cj/jshintmojo/EmbeddedVersionsTest.java index d5e0bf9..09c700f 100644 --- a/src/test/java/com/cj/jshintmojo/EmbeddedVersionsTest.java +++ b/src/test/java/com/cj/jshintmojo/EmbeddedVersionsTest.java @@ -7,6 +7,7 @@ import java.util.List; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -14,7 +15,7 @@ import com.cj.jshintmojo.jshint.EmbeddedJshintCode; import com.cj.jshintmojo.jshint.JSHint; -import com.cj.jshintmojo.jshint.JSHint.Error; +import com.cj.jshintmojo.jshint.JSHint.Hint; @RunWith(Parameterized.class) public class EmbeddedVersionsTest { @@ -40,7 +41,7 @@ public static Collection data() { private final String jshintVersion; private final OutputMessagesVariant variants; - public EmbeddedVersionsTest(String jshintVersion) { + public EmbeddedVersionsTest(final String jshintVersion) { super(); this.jshintVersion = EmbeddedJshintCode.EMBEDDED_VERSIONS.get(jshintVersion); @@ -62,7 +63,7 @@ protected String expectedEvalIsEvilMessage() { } - + @Ignore("Because 'code' is null") @Test public void booleanOptionsCanBeFalse(){ // given @@ -72,12 +73,12 @@ public void booleanOptionsCanBeFalse(){ final JSHint jsHint = new JSHint(jshintVersion); // when - List errors = jsHint.run(code, options, globals); + List hints = jsHint.run(code, options, globals); // then - Assert.assertNotNull(errors); - Assert.assertEquals(1, errors.size()); - Assert.assertEquals(variants.expectedEvalIsEvilMessage(), errors.get(0).reason); + Assert.assertNotNull(hints); + Assert.assertEquals(1, hints.size()); + Assert.assertEquals(variants.expectedEvalIsEvilMessage(), hints.get(0).reason); } @@ -91,13 +92,14 @@ public void booleanOptionsCanBeTrue(){ final JSHint jsHint = new JSHint(jshintVersion); // when - List errors = jsHint.run(code, options, globals); + List hints = jsHint.run(code, options, globals); // then - Assert.assertNotNull(errors); - Assert.assertEquals(0, errors.size()); + Assert.assertNotNull(hints); + Assert.assertEquals(0, hints.size()); } + @Ignore("Because 'code' is null") @Test public void supportsOptionsThatTakeANumericValue(){ // given @@ -107,14 +109,15 @@ public void supportsOptionsThatTakeANumericValue(){ final JSHint jsHint = new JSHint(jshintVersion); // when - List errors = jsHint.run(code, options, globals); + List hints = jsHint.run(code, options, globals); // then - Assert.assertNotNull(errors); - Assert.assertEquals(1, errors.size()); - Assert.assertEquals("Expected 'alert' to have an indentation at 1 instead at 2.", errors.get(0).reason); + Assert.assertNotNull(hints); + Assert.assertEquals(1, hints.size()); + Assert.assertEquals("Expected 'alert' to have an indentation at 1 instead at 2.", hints.get(0).reason); } + @Ignore("Because 'code' is null") @Test public void supportsParametersWithValues(){ // given @@ -124,14 +127,15 @@ public void supportsParametersWithValues(){ final JSHint jsHint = new JSHint(jshintVersion); // when - List errors = jsHint.run(code, options, globals); + List hints = jsHint.run(code, options, globals); // then - Assert.assertNotNull(errors); - Assert.assertEquals(1, errors.size()); - Assert.assertEquals(variants.expectedErrorMessageForTwoTooManyParameters(), errors.get(0).reason); + Assert.assertNotNull(hints); + Assert.assertEquals(1, hints.size()); + Assert.assertEquals(variants.expectedErrorMessageForTwoTooManyParameters(), hints.get(0).reason); } + @Ignore("Because 'code' is null") @Test public void supportsParametersWithoutValues(){ // given @@ -141,12 +145,13 @@ public void supportsParametersWithoutValues(){ final JSHint jsHint = new JSHint(jshintVersion); // when - List errors = jsHint.run(code, options, globals); + List hints = jsHint.run(code, options, globals); // then - Assert.assertNotNull(errors); - Assert.assertEquals(1, errors.size()); - Assert.assertEquals("Do not use 'new' for side effects.", errors.get(0).raw); + Assert.assertNotNull(hints); + Assert.assertEquals(1, hints.size()); + Assert.assertEquals("-W031", hints.get(0).code); + Assert.assertEquals("Do not use 'new' for side effects.", hints.get(0).raw); } @Test @@ -158,21 +163,21 @@ public void supportsTheGlobalsParameter(){ final JSHint jsHint = new JSHint(jshintVersion); // when - List errors = jsHint.run(code, options, globals); + List hints = jsHint.run(code, options, globals); // then - Assert.assertNotNull(errors); - Assert.assertEquals("Expected no errors, but received:\n " + toString(errors), 0, errors.size()); + Assert.assertNotNull(hints); + Assert.assertEquals("Expected no hints, but received:\n " + toString(hints), 0, hints.size()); } - private static InputStream toStream(String text){ + private static InputStream toStream(final String text){ return new ByteArrayInputStream(text.getBytes()); } - private static String toString(List errors) { + private static String toString(final List hints) { StringBuffer text = new StringBuffer(); - for(Error error: errors){ - text.append(error.reason + "\n"); + for(Hint hint: hints){ + text.append(hint.reason + "\n"); } return text.toString(); } diff --git a/src/test/java/com/cj/jshintmojo/MojoTest.java b/src/test/java/com/cj/jshintmojo/MojoTest.java index b5b4b5c..bfb9c9a 100644 --- a/src/test/java/com/cj/jshintmojo/MojoTest.java +++ b/src/test/java/com/cj/jshintmojo/MojoTest.java @@ -1,59 +1,57 @@ package com.cj.jshintmojo; -import static com.cj.jshintmojo.util.Util.*; -import static org.apache.commons.io.FileUtils.writeStringToFile; -import static org.junit.Assert.*; +import static com.cj.jshintmojo.util.Util.deleteDirectory; +import static com.cj.jshintmojo.util.Util.mkdirs; +import static com.cj.jshintmojo.util.Util.tempDir; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.io.File; import java.util.Arrays; import java.util.Collections; -import java.util.List; import org.apache.commons.io.FileUtils; -import org.apache.commons.io.IOUtils; import org.junit.Test; public class MojoTest { - - @Test public void walksTheDirectoryTreeToFindAndUseJshintFiles() throws Exception { String[] jshintIgnorePaths = {".jshintignore", "foo/.jshintignore", "foo/bar/.jshintignore", "foo/bar/baz/.jshintignore"}; - + for(String jshintIgnorePath: jshintIgnorePaths){ File directory = tempDir(); try{ // given File ignoreFile = new File(directory, jshintIgnorePath); FileUtils.writeStringToFile(ignoreFile, "src/main/resources/foo.qunit.js"); - + File projectDirectory = mkdirs(directory, "foo/bar/baz"); File resourcesDirectory = mkdirs(projectDirectory, "src/main/resources"); - + File fileToIgnore = new File(resourcesDirectory, "foo.qunit.js"); FileUtils.writeStringToFile(fileToIgnore, "whatever, this should be ignored"); - + LogStub log = new LogStub(); - Mojo mojo = new Mojo("", "", - projectDirectory, - Collections.singletonList("src/main/resources"), - Collections.emptyList(),true, null, null, null, null); + Mojo mojo = new Mojo("", "", + projectDirectory, + Collections.singletonList("src/main/resources"), + Collections.emptyList(), true, true, null, null, null, null); mojo.setLog(log); - + // when mojo.execute(); - + // then assertTrue("Sees ignore files", log.hasMessage("info", "Using ignore file: " + ignoreFile.getAbsolutePath())); - assertTrue("Uses ignore files", log.hasMessage("warn", "Excluding " + fileToIgnore.getAbsolutePath())); - + assertTrue("Uses ignore files", log.hasMessage("info", "Excluding " + fileToIgnore.getAbsolutePath())); + }finally{ deleteDirectory(directory); } } } - + @Test public void warnsUsersWhenConfiguredToWorkWithNonexistentDirectories() throws Exception { File directory = tempDir(); @@ -61,58 +59,94 @@ public void warnsUsersWhenConfiguredToWorkWithNonexistentDirectories() throws Ex // given mkdirs(directory, "src/main/resources"); LogStub log = new LogStub(); - Mojo mojo = new Mojo("", "", - directory, - Collections.singletonList("src/main/resources/nonexistentDirectory"), - Collections.emptyList(),true, null, null, null, null); + Mojo mojo = new Mojo("", "", + directory, + Collections.singletonList("src/main/resources/nonexistentDirectory"), + Collections.emptyList(),true, true, null, null, null, null); mojo.setLog(log); // when mojo.execute(); // then - assertEquals(1, log.messagesForLevel("warn").size()); + assertEquals(0, log.messagesForLevel("warn").size()); + assertEquals(2, log.messagesForLevel("info").size()); assertEquals("You told me to find tests in src/main/resources/nonexistentDirectory, but there is nothing there (" + directory.getAbsolutePath() + File.separator + "src" + File.separator + "main" + File.separator + "resources" + File.separator + "nonexistentDirectory)", - log.messagesForLevel("warn").get(0).content.toString()); + log.messagesForLevel("info").get(1).content.toString()); }finally{ deleteDirectory(directory); } } - + @Test - public void resolvesConfigFileRelativeToMavenBasedirProperty() throws Exception { - File directory = tempDir(); - try{ - // given - mkdirs(directory, "src/main/resources"); - mkdirs(directory, "foo/bar"); - FileUtils.writeLines(new File(directory, "foo/bar/my-config-file.js"), Arrays.asList( - "{", - " \"globals\": {", - " \"require\": false", - " }", - "}" - )); - - Mojo mojo = new Mojo(null, "", - directory, - Collections.singletonList("src/main/resources/"), - Collections.emptyList(),true, "foo/bar/my-config-file.js", null, null, null); - - LogStub log = new LogStub(); - mojo.setLog(log); + public void resolvesConfigFileRelativeToMavenBasedirProperty() throws Exception { + File directory = tempDir(); + try{ + // given + mkdirs(directory, "src/main/resources"); + mkdirs(directory, "foo/bar"); + FileUtils.writeLines(new File(directory, "foo/bar/my-config-file.js"), Arrays.asList( + "{", + " \"globals\": {", + " \"require\": false", + " }", + "}" + )); + + Mojo mojo = new Mojo(null, "", + directory, + Collections.singletonList("src/main/resources/"), + Collections.emptyList(), true, true, "foo/bar/my-config-file.js", null, null, null); + + LogStub log = new LogStub(); + mojo.setLog(log); - // when - mojo.execute(); - - // then - final String properPathForConfigFile = new File(directory, "foo/bar/my-config-file.js").getAbsolutePath(); - assertTrue(log.hasMessage("info", "Using configuration file: " + properPathForConfigFile)); - - }finally{ - deleteDirectory(directory); - } - } + // when + mojo.execute(); + + // then + final String properPathForConfigFile = new File(directory, "foo/bar/my-config-file.js").getAbsolutePath(); + assertTrue(log.hasMessage("info", "Using configured 'configFile' from: " + properPathForConfigFile)); + + }finally{ + deleteDirectory(directory); + } + } + + @Test + public void resolvesConfigFileByAbsolutePath() throws Exception { + File directory = tempDir(); + File baseDir = tempDir(); + try{ + // given + mkdirs(directory, "src/main/resources"); + mkdirs(directory, "foo/bar"); + File jshintConf = new File(directory, "foo/bar/jshint.conf.json"); + FileUtils.writeLines(jshintConf, Arrays.asList( + "{", + " \"globals\": {", + " \"require\": false", + " }", + "}" + )); + + Mojo mojo = new Mojo(null, "", + baseDir, + Collections.singletonList("src/main/resources/"), + Collections.emptyList(), true, true, jshintConf.getAbsolutePath(), null, null, null); + + LogStub log = new LogStub(); + mojo.setLog(log); + + // when + mojo.execute(); + + // then + assertTrue(log.hasMessage("info", "Using configured 'configFile' from: " + jshintConf.getAbsolutePath())); + }finally{ + deleteDirectory(directory); + } + } } diff --git a/src/test/java/com/cj/jshintmojo/reporter/CheckStyleReporterTest.java b/src/test/java/com/cj/jshintmojo/reporter/CheckStyleReporterTest.java index 71eba54..18e55b0 100644 --- a/src/test/java/com/cj/jshintmojo/reporter/CheckStyleReporterTest.java +++ b/src/test/java/com/cj/jshintmojo/reporter/CheckStyleReporterTest.java @@ -1,7 +1,7 @@ package com.cj.jshintmojo.reporter; -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; import org.junit.Test; @@ -41,9 +41,9 @@ public void reportSingleFileFailedResults() { "\n" + "\n" + "\t\n" + - "\t\t\n" + - "\t\t\n" + - "\t\t\n" + + "\t\t\n" + + "\t\t\n" + + "\t\t\n" + "\t\n" + "\n")); } @@ -56,16 +56,16 @@ public void reportMultipleFilesFailedResults() { "\n" + "\n" + "\t\n" + - "\t\t\n" + - "\t\t\n" + - "\t\t\n" + + "\t\t\n" + + "\t\t\n" + + "\t\t\n" + "\t\n" + "\t\n" + - "\t\t\n" + + "\t\t\n" + "\t\n" + "\t\n" + - "\t\t\n" + - "\t\t\n" + + "\t\t\n" + + "\t\t\n" + "\t\n" + "\n")); } diff --git a/src/test/java/com/cj/jshintmojo/reporter/JSHintReporterTestUtil.java b/src/test/java/com/cj/jshintmojo/reporter/JSHintReporterTestUtil.java index a82af67..d9138a3 100644 --- a/src/test/java/com/cj/jshintmojo/reporter/JSHintReporterTestUtil.java +++ b/src/test/java/com/cj/jshintmojo/reporter/JSHintReporterTestUtil.java @@ -7,6 +7,7 @@ import com.cj.jshintmojo.cache.Result; import com.cj.jshintmojo.jshint.JSHint; +import com.cj.jshintmojo.jshint.JSHint.Hint; /** * Utility class to create test data for {@link JSHintReporter}. @@ -16,20 +17,33 @@ class JSHintReporterTestUtil { private JSHintReporterTestUtil() { } - private static JSHint.Error createError(String code, int line, - int character, String evidence, String reason) - { - JSHint.Error error = new JSHint.Error(); - error.id = "(error)"; - error.code = code; - error.line = line; - error.character = character; - error.evidence = evidence; - error.reason = reason; - error.raw = reason; - return error; + private static JSHint.Hint createHint(final String code, final int line, + final int character, final String evidence, final String reason) { + JSHint.Hint hint; + char c = code.charAt(0); + switch (c) { + case 'W': + hint = new JSHint.Warning(); + break; + case 'E': + hint = new JSHint.Error(); + break; + case 'I': + hint = new JSHint.Info(); + break; + default: + throw new IllegalStateException(); + } + hint.id = "(error)"; + hint.code = code; + hint.line = line; + hint.character = character; + hint.evidence = evidence; + hint.reason = reason; + hint.raw = reason; + return hint; } - + static Map createAllPassedResults() { return new HashMap(); } @@ -37,11 +51,11 @@ static Map createAllPassedResults() { static Map createSingleFileFailedResults() { Map results = new HashMap(); { - List errors = new ArrayList(); - errors.add(createError("W033", 5, 2, "}", "Missing semicolon.")); - errors.add(createError("W014", 12, 5, " && window.setImmediate;", "Bad line breaking before '&&'.")); - errors.add(createError("E043", 1137, 26, null, "Too many errors.")); - Result result = new Result("/path/to/A.js", 1377309240000L, errors); + List hints = new ArrayList(); + hints.add(createHint("W033", 5, 2, "}", "Missing semicolon.")); + hints.add(createHint("W014", 12, 5, " && window.setImmediate;", "Bad line breaking before '&&'.")); + hints.add(createHint("E043", 1137, 26, null, "Too many hints.")); + Result result = new Result("/path/to/A.js", 1377309240000L, hints); results.put(result.path, result); } return results; @@ -50,24 +64,24 @@ static Map createSingleFileFailedResults() { static Map createMultipleFilesFailedResults() { Map results = new HashMap(); { - List errors = new ArrayList(); - errors.add(createError("W033", 5, 2, "}", "Missing semicolon.")); - errors.add(createError("W014", 12, 5, " && window.setImmediate;", "Bad line breaking before '&&'.")); - errors.add(createError("E043", 1137, 26, null, "Too many errors.")); - Result result = new Result("/path/to/A.js", 1377309240000L, errors); + List hints = new ArrayList(); + hints.add(createHint("W033", 5, 2, "}", "Missing semicolon.")); + hints.add(createHint("W014", 12, 5, " && window.setImmediate;", "Bad line breaking before '&&'.")); + hints.add(createHint("E043", 1137, 26, null, "Too many hints.")); + Result result = new Result("/path/to/A.js", 1377309240000L, hints); results.put(result.path, result); } { - List errors = new ArrayList(); - errors.add(createError("I001", 10, 5, " , [info, \"info\"]", "Comma warnings can be turned off with 'laxcomma'.")); - Result result = new Result("/path/to/B.js", 1377309240000L, errors); + List hints = new ArrayList(); + hints.add(createHint("I001", 10, 5, " , [info, \"info\"]", "Comma warnings can be turned off with 'laxcomma'.")); + Result result = new Result("/path/to/B.js", 1377309240000L, hints); results.put(result.path, result); } { - List errors = new ArrayList(); - errors.add(createError("W004", 3, 14, " var args = a;", "'args' is already defined.")); - errors.add(createError("W041", 12, 5, " if (list.length == 0)", "Use '===' to compare with '0'.")); - Result result = new Result("/path/to/C.js", 1377309240000L, errors); + List hints = new ArrayList(); + hints.add(createHint("W004", 3, 14, " var args = a;", "'args' is already defined.")); + hints.add(createHint("W041", 12, 5, " if (list.length == 0)", "Use '===' to compare with '0'.")); + Result result = new Result("/path/to/C.js", 1377309240000L, hints); results.put(result.path, result); } return results; diff --git a/src/test/java/com/cj/jshintmojo/reporter/JSLintReporterTest.java b/src/test/java/com/cj/jshintmojo/reporter/JSLintReporterTest.java index b429f23..7c28b02 100644 --- a/src/test/java/com/cj/jshintmojo/reporter/JSLintReporterTest.java +++ b/src/test/java/com/cj/jshintmojo/reporter/JSLintReporterTest.java @@ -1,7 +1,7 @@ package com.cj.jshintmojo.reporter; -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; import org.junit.Test; @@ -43,7 +43,7 @@ public void reportSingleFileFailedResults() { "\t\n" + "\t\t\n" + "\t\t\n" + - "\t\t\n" + + "\t\t\n" + "\t\n" + "\n")); } @@ -58,7 +58,7 @@ public void reportMultipleFilesFailedResults() { "\t\n" + "\t\t\n" + "\t\t\n" + - "\t\t\n" + + "\t\t\n" + "\t\n" + "\t\n" + "\t\t\n" +