From 327a0996dcda6f29d73844e795ac128e43ebcb3b Mon Sep 17 00:00:00 2001 From: Sebl29 Date: Tue, 8 Apr 2014 19:17:34 +0200 Subject: [PATCH 1/5] Added warnings; compiles and tests are green. --- .../java/com/cj/jshintmojo/DumpCacheMojo.java | 15 +- src/main/java/com/cj/jshintmojo/Mojo.java | 143 ++++++++++-------- .../java/com/cj/jshintmojo/cache/Result.java | 10 +- .../java/com/cj/jshintmojo/jshint/JSHint.java | 67 +++++--- .../reporter/CheckStyleReporter.java | 16 +- .../jshintmojo/reporter/JSLintReporter.java | 16 +- .../cj/jshintmojo/EmbeddedVersionsTest.java | 56 +++---- src/test/java/com/cj/jshintmojo/MojoTest.java | 67 ++++---- .../reporter/CheckStyleReporterTest.java | 8 +- .../reporter/JSHintReporterTestUtil.java | 39 ++--- .../reporter/JSLintReporterTest.java | 8 +- 11 files changed, 241 insertions(+), 204 deletions(-) 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..f44bedd 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; @@ -31,12 +32,13 @@ 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.jshint.JSHint.Warning; 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 +63,7 @@ public class Mojo extends AbstractMojo { private String options = null; /** - * @parameter + * @parameter */ private String globals = ""; @@ -88,10 +90,10 @@ 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; @@ -101,10 +103,10 @@ 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 String configFile, final String reporter, final String reportFile, final String ignoreFile) { super(); this.options = options; this.globals = globals; @@ -117,12 +119,13 @@ public Mojo(String options, String globals, File basedir, List directori 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,45 +133,45 @@ 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 { + + private static Config readConfig(final String options, final String globals, final String configFileParam, final File basedir, final Log log) throws MojoExecutionException { final File jshintRc = findJshintrc(basedir); final File configFile = StringUtils.isNotBlank(configFileParam)?new File(basedir, configFileParam):null; - + final Config config; if(options==null){ if(configFile!=null){ @@ -183,7 +186,7 @@ private static Config readConfig(String options, String globals, String configFi }else{ config = new Config(options, globals); } - + return config; } @@ -191,13 +194,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 +224,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,26 +247,30 @@ 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); + 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(Error error: r.errors){ - log.error(" " + error.line.intValue() + "," + error.character.intValue() + ": " + error.reason); + for(Hint hint: r.hints) { + if (hint instanceof Warning) { + log.warn(" " + hint.line.intValue() + "," + hint.character.intValue() + ": " + hint.reason); + } else if (hint instanceof Error) { + log.error(" " + hint.line.intValue() + "," + hint.character.intValue() + ": " + hint.reason); + } } } } @@ -270,14 +278,13 @@ private static Map lintTheFiles(final JSHint jshint, final Cache } 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; for(Result r : currentResults.values()){ - if(!r.errors.isEmpty()){ + if(!r.hints.isEmpty()){ numProblematicFiles ++; errorRecap @@ -285,19 +292,19 @@ private void handleResults(final Map currentResults, .append(r.path) .append(NEWLINE); - for(Error error: r.errors){ + for (Hint hint: r.hints) { errorRecap .append(" ") - .append(error.line.intValue()) + .append(hint.line.intValue()) .append(",") - .append(error.character.intValue()) + .append(hint.character.intValue()) .append(": ") - .append(error.reason) + .append(hint.reason) .append(NEWLINE); } } } - + if(numProblematicFiles > 0) { saveReportFile(currentResults, reporter, reportFile); @@ -318,7 +325,7 @@ private void handleResults(final Map currentResults, } } - 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 +361,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 +373,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 +384,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 +402,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 +422,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 +446,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 +458,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 +470,7 @@ private static Config processConfigFile(File configFile) throws MojoExecutionExc }else{ options = ""; } - + return new Config(options, globals); } @@ -469,7 +480,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..712a1f1 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); @@ -60,7 +61,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 +83,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 +100,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 +134,29 @@ public Error(JSObject o) { reason = o.dot("reason"); } + public Hint() { } + + } + + @SuppressWarnings("serial") + public static class Warning extends Hint implements Serializable { + + public Warning(final JSObject o) { + super(o); + } + // NOTE: for Unit Testing purpose. - public Error() { + 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() { } } } diff --git a/src/main/java/com/cj/jshintmojo/reporter/CheckStyleReporter.java b/src/main/java/com/cj/jshintmojo/reporter/CheckStyleReporter.java index e563a37..e924090 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){ + for (Hint hint : result.hints) { buf.append(String.format("\t\t\n", - error.line.intValue(), error.character.intValue(), encode(error.reason), encode(error.code), severity(error.code))); + hint.line.intValue(), hint.character.intValue(), encode(hint.reason), encode(hint.code), severity(hint.code))); } buf.append("\t\n"); } @@ -41,8 +41,8 @@ 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': @@ -56,8 +56,8 @@ private String severity(String errorCode) { } return "warning"; } - - 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..a8922ba 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..ced88e0 100644 --- a/src/test/java/com/cj/jshintmojo/EmbeddedVersionsTest.java +++ b/src/test/java/com/cj/jshintmojo/EmbeddedVersionsTest.java @@ -14,7 +14,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 +40,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); @@ -72,12 +72,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,11 +91,11 @@ 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()); } @Test @@ -107,12 +107,12 @@ 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); } @Test @@ -124,12 +124,12 @@ 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); } @Test @@ -141,12 +141,12 @@ 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("Do not use 'new' for side effects.", hints.get(0).raw); } @Test @@ -158,21 +158,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..36e0e57 100644 --- a/src/test/java/com/cj/jshintmojo/MojoTest.java +++ b/src/test/java/com/cj/jshintmojo/MojoTest.java @@ -1,59 +1,59 @@ 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"), + Mojo mojo = new Mojo("", "", + projectDirectory, + Collections.singletonList("src/main/resources"), Collections.emptyList(),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,9 +61,9 @@ 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"), + Mojo mojo = new Mojo("", "", + directory, + Collections.singletonList("src/main/resources/nonexistentDirectory"), Collections.emptyList(),true, null, null, null, null); mojo.setLog(log); @@ -71,16 +71,17 @@ public void warnsUsersWhenConfiguredToWorkWithNonexistentDirectories() throws Ex 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(); @@ -90,27 +91,27 @@ public void resolvesConfigFileRelativeToMavenBasedirProperty() throws Exception mkdirs(directory, "foo/bar"); FileUtils.writeLines(new File(directory, "foo/bar/my-config-file.js"), Arrays.asList( "{", - " \"globals\": {", + " \"globals\": {", " \"require\": false", - " }", + " }", "}" )); - - Mojo mojo = new Mojo(null, "", - directory, - Collections.singletonList("src/main/resources/"), + + 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); // 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); } diff --git a/src/test/java/com/cj/jshintmojo/reporter/CheckStyleReporterTest.java b/src/test/java/com/cj/jshintmojo/reporter/CheckStyleReporterTest.java index 71eba54..70a2a0c 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; @@ -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" + diff --git a/src/test/java/com/cj/jshintmojo/reporter/JSHintReporterTestUtil.java b/src/test/java/com/cj/jshintmojo/reporter/JSHintReporterTestUtil.java index a82af67..f2f5941 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,8 +17,8 @@ class JSHintReporterTestUtil { private JSHintReporterTestUtil() { } - private static JSHint.Error createError(String code, int line, - int character, String evidence, String reason) + private static JSHint.Error createError(final String code, final int line, + final int character, final String evidence, final String reason) { JSHint.Error error = new JSHint.Error(); error.id = "(error)"; @@ -37,11 +38,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(createError("W033", 5, 2, "}", "Missing semicolon.")); + hints.add(createError("W014", 12, 5, " && window.setImmediate;", "Bad line breaking before '&&'.")); + hints.add(createError("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 +51,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(createError("W033", 5, 2, "}", "Missing semicolon.")); + hints.add(createError("W014", 12, 5, " && window.setImmediate;", "Bad line breaking before '&&'.")); + hints.add(createError("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(createError("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(createError("W004", 3, 14, " var args = a;", "'args' is already defined.")); + hints.add(createError("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" + From ac3e32bb3d87c06179cd4e82c701493df40a685f Mon Sep 17 00:00:00 2001 From: Sebl29 Date: Wed, 9 Apr 2014 19:12:06 +0200 Subject: [PATCH 2/5] parsing response --- src/main/java/com/cj/jshintmojo/jshint/JSHint.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/cj/jshintmojo/jshint/JSHint.java b/src/main/java/com/cj/jshintmojo/jshint/JSHint.java index 712a1f1..c194cdb 100644 --- a/src/main/java/com/cj/jshintmojo/jshint/JSHint.java +++ b/src/main/java/com/cj/jshintmojo/jshint/JSHint.java @@ -52,8 +52,12 @@ public List run(final InputStream source, final String options, final Stri 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); + JSObject jso = new JSObject(next); + if (jso.dot("id").toString().equals("(error)")) { + results.add(new Error(jso)); + } else if (jso.toString().startsWith("(warning)")) { + results.add(new Warning(jso)); + } } } } From 6f9d0679e9318853ddcc7342bd23ad0608fe2dba Mon Sep 17 00:00:00 2001 From: Sebl29 Date: Wed, 30 Apr 2014 09:27:55 +0200 Subject: [PATCH 3/5] added info. Instances of JSHint.Hint now written to report. All tests passing. --- src/main/java/com/cj/jshintmojo/jshint/JSHint.java | 13 +++++++++++++ .../cj/jshintmojo/reporter/CheckStyleReporter.java | 6 ++++-- .../reporter/CheckStyleReporterTest.java | 14 +++++++------- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/cj/jshintmojo/jshint/JSHint.java b/src/main/java/com/cj/jshintmojo/jshint/JSHint.java index c194cdb..467bbfd 100644 --- a/src/main/java/com/cj/jshintmojo/jshint/JSHint.java +++ b/src/main/java/com/cj/jshintmojo/jshint/JSHint.java @@ -57,6 +57,8 @@ public List run(final InputStream source, final String options, final Stri results.add(new Error(jso)); } else if (jso.toString().startsWith("(warning)")) { results.add(new Warning(jso)); + } else if (jso.toString().startsWith("(info)")) { + results.add(new Info(jso)); } } } @@ -163,4 +165,15 @@ public Error(final JSObject o) { // NOTE: for Unit Testing purpose. 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 e924090..bfac03a 100644 --- a/src/main/java/com/cj/jshintmojo/reporter/CheckStyleReporter.java +++ b/src/main/java/com/cj/jshintmojo/reporter/CheckStyleReporter.java @@ -6,6 +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; /** @@ -32,8 +33,8 @@ public String report(final Map results) { Result result = results.get(file); buf.append("\t\n"); for (Hint hint : result.hints) { - buf.append(String.format("\t\t\n", - hint.line.intValue(), hint.character.intValue(), encode(hint.reason), encode(hint.code), severity(hint.code))); + 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"); } @@ -50,6 +51,7 @@ private String severity(final String errorCode) { case 'I': return "info"; case 'W': + return "warning"; default: break; } diff --git a/src/test/java/com/cj/jshintmojo/reporter/CheckStyleReporterTest.java b/src/test/java/com/cj/jshintmojo/reporter/CheckStyleReporterTest.java index 70a2a0c..18e55b0 100644 --- a/src/test/java/com/cj/jshintmojo/reporter/CheckStyleReporterTest.java +++ b/src/test/java/com/cj/jshintmojo/reporter/CheckStyleReporterTest.java @@ -41,8 +41,8 @@ public void reportSingleFileFailedResults() { "\n" + "\n" + "\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\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")); } From 65715b2503c0ea8b729ca169aa44155bb7828d43 Mon Sep 17 00:00:00 2001 From: Sebl29 Date: Thu, 1 May 2014 18:36:16 +0200 Subject: [PATCH 4/5] JSHint Errors/Warnings now seperated. Added config value 'failOnWarning'. All tests green. Reduced the duplicated jshint messages on mojo-fail, as the 'ERROR' word is misleading (left side, from logger). Will now request a pull. --- README.md | 6 +- pom.xml | 87 ++++----- src/main/java/com/cj/jshintmojo/Mojo.java | 168 +++++++++++------- .../java/com/cj/jshintmojo/jshint/JSHint.java | 37 +++- .../jshintmojo/reporter/JSLintReporter.java | 2 +- .../cj/jshintmojo/EmbeddedVersionsTest.java | 7 +- src/test/java/com/cj/jshintmojo/MojoTest.java | 142 +++++++++++---- 7 files changed, 297 insertions(+), 152 deletions(-) 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/Mojo.java b/src/main/java/com/cj/jshintmojo/Mojo.java index f44bedd..bd95708 100644 --- a/src/main/java/com/cj/jshintmojo/Mojo.java +++ b/src/main/java/com/cj/jshintmojo/Mojo.java @@ -31,9 +31,7 @@ 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.jshint.JSHint.Warning; import com.cj.jshintmojo.reporter.CheckStyleReporter; import com.cj.jshintmojo.reporter.JSHintReporter; import com.cj.jshintmojo.reporter.JSLintReporter; @@ -96,6 +94,10 @@ public class Mojo extends AbstractMojo { * @parameter */ private Boolean failOnError = true; + /** + * @parameter + */ + private Boolean failOnWarning = true; /** * @parameter default-value="${basedir} @@ -106,14 +108,17 @@ public class Mojo extends AbstractMojo { public Mojo() {} - public Mojo(final String options, final String globals, final File basedir, final List directories, final List excludes, final boolean failOnError, final String configFile, final String reporter, final String reportFile, final 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; @@ -169,24 +174,31 @@ public Config(final String options, final String globals) { } private static Config readConfig(final String options, final String globals, final String configFileParam, final File basedir, final Log log) throws MojoExecutionException { - final File jshintRc = findJshintrc(basedir); - final File configFile = StringUtils.isNotBlank(configFileParam)?new File(basedir, configFileParam):null; - 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; } @@ -249,30 +261,33 @@ public Boolean apply(final File i) { 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 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; - } + 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) { - if (hint instanceof Warning) { - log.warn(" " + hint.line.intValue() + "," + hint.character.intValue() + ": " + hint.reason); - } else if (hint instanceof Error) { - log.error(" " + hint.line.intValue() + "," + hint.character.intValue() + ": " + hint.reason); + 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; } @@ -283,23 +298,27 @@ private void handleResults(final Map currentResults, StringBuilder errorRecap = new StringBuilder(NEWLINE); int numProblematicFiles = 0; + boolean hasErrors = false; + boolean hasWarnings = false; for(Result r : currentResults.values()){ - if(!r.hints.isEmpty()){ - numProblematicFiles ++; + if (!r.hints.isEmpty()) { + numProblematicFiles ++; errorRecap .append(NEWLINE) .append(r.path) .append(NEWLINE); - for (Hint hint: r.hints) { - errorRecap - .append(" ") - .append(hint.line.intValue()) - .append(",") - .append(hint.character.intValue()) - .append(": ") - .append(hint.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); } } @@ -308,20 +327,43 @@ private void handleResults(final Map currentResults, 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); + } } } diff --git a/src/main/java/com/cj/jshintmojo/jshint/JSHint.java b/src/main/java/com/cj/jshintmojo/jshint/JSHint.java index 467bbfd..6b6b946 100644 --- a/src/main/java/com/cj/jshintmojo/jshint/JSHint.java +++ b/src/main/java/com/cj/jshintmojo/jshint/JSHint.java @@ -51,14 +51,10 @@ public List run(final InputStream source, final String options, final Stri 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 + 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(new Error(jso)); - } else if (jso.toString().startsWith("(warning)")) { - results.add(new Warning(jso)); - } else if (jso.toString().startsWith("(info)")) { - results.add(new Info(jso)); + results.add(Hint.createHint(jso)); } } } @@ -141,6 +137,35 @@ public Hint(final JSObject o) { } 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; + } } diff --git a/src/main/java/com/cj/jshintmojo/reporter/JSLintReporter.java b/src/main/java/com/cj/jshintmojo/reporter/JSLintReporter.java index a8922ba..4d9c15f 100644 --- a/src/main/java/com/cj/jshintmojo/reporter/JSLintReporter.java +++ b/src/main/java/com/cj/jshintmojo/reporter/JSLintReporter.java @@ -31,7 +31,7 @@ public String report(final Map results) { for(String file : files){ Result result = results.get(file); buf.append("\t\n"); - for(Hint hint : result.hints){ + for (Hint hint : result.hints) { buf.append(String.format("\t\temptyList(),true, null, null, null, null); + Collections.emptyList(), true, true, null, null, null, null); mojo.setLog(log); // when @@ -64,7 +62,7 @@ public void warnsUsersWhenConfiguredToWorkWithNonexistentDirectories() throws Ex Mojo mojo = new Mojo("", "", directory, Collections.singletonList("src/main/resources/nonexistentDirectory"), - Collections.emptyList(),true, null, null, null, null); + Collections.emptyList(),true, true, null, null, null, null); mojo.setLog(log); // when @@ -82,38 +80,108 @@ public void warnsUsersWhenConfiguredToWorkWithNonexistentDirectories() throws Ex } } +// @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, 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); +// } +// } + @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); - - // 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); - } - } + 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 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); + } + } } From 4019e144ef3aa96edbdd32ec5d7ab9e25b95fef2 Mon Sep 17 00:00:00 2001 From: Sebl29 Date: Thu, 1 May 2014 19:19:57 +0200 Subject: [PATCH 5/5] JSHint Errors/Warnings now seperated. Added config value 'failOnWarning'. All tests green. Reduced the duplicated jshint messages on mojo-fail, as the 'ERROR' word is misleading (left side, from logger). Will now request a pull. --- .../reporter/CheckStyleReporter.java | 9 +-- src/test/java/com/cj/jshintmojo/MojoTest.java | 35 ------------ .../reporter/JSHintReporterTestUtil.java | 57 ++++++++++++------- 3 files changed, 38 insertions(+), 63 deletions(-) diff --git a/src/main/java/com/cj/jshintmojo/reporter/CheckStyleReporter.java b/src/main/java/com/cj/jshintmojo/reporter/CheckStyleReporter.java index bfac03a..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,6 @@ 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; /** @@ -48,15 +47,13 @@ private String severity(final String errorCode) { switch(errorCode.charAt(0)){ case 'E': return "error"; - case 'I': - return "info"; case 'W': return "warning"; - default: - break; + case 'I': + return "info"; } } - return "warning"; + throw new IllegalArgumentException(); } private String encode(final String str) { diff --git a/src/test/java/com/cj/jshintmojo/MojoTest.java b/src/test/java/com/cj/jshintmojo/MojoTest.java index b9f1c97..bfb9c9a 100644 --- a/src/test/java/com/cj/jshintmojo/MojoTest.java +++ b/src/test/java/com/cj/jshintmojo/MojoTest.java @@ -80,41 +80,6 @@ public void warnsUsersWhenConfiguredToWorkWithNonexistentDirectories() throws Ex } } -// @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, 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); -// } -// } - @Test public void resolvesConfigFileRelativeToMavenBasedirProperty() throws Exception { File directory = tempDir(); diff --git a/src/test/java/com/cj/jshintmojo/reporter/JSHintReporterTestUtil.java b/src/test/java/com/cj/jshintmojo/reporter/JSHintReporterTestUtil.java index f2f5941..d9138a3 100644 --- a/src/test/java/com/cj/jshintmojo/reporter/JSHintReporterTestUtil.java +++ b/src/test/java/com/cj/jshintmojo/reporter/JSHintReporterTestUtil.java @@ -17,20 +17,33 @@ class JSHintReporterTestUtil { private JSHintReporterTestUtil() { } - private static JSHint.Error createError(final String code, final int line, - final int character, final String evidence, final 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(); } @@ -39,9 +52,9 @@ static Map createSingleFileFailedResults() { Map results = new HashMap(); { List hints = new ArrayList(); - hints.add(createError("W033", 5, 2, "}", "Missing semicolon.")); - hints.add(createError("W014", 12, 5, " && window.setImmediate;", "Bad line breaking before '&&'.")); - hints.add(createError("E043", 1137, 26, null, "Too many hints.")); + 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); } @@ -52,22 +65,22 @@ static Map createMultipleFilesFailedResults() { Map results = new HashMap(); { List hints = new ArrayList(); - hints.add(createError("W033", 5, 2, "}", "Missing semicolon.")); - hints.add(createError("W014", 12, 5, " && window.setImmediate;", "Bad line breaking before '&&'.")); - hints.add(createError("E043", 1137, 26, null, "Too many hints.")); + 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 hints = new ArrayList(); - hints.add(createError("I001", 10, 5, " , [info, \"info\"]", "Comma warnings can be turned off with 'laxcomma'.")); + 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 hints = new ArrayList(); - hints.add(createError("W004", 3, 14, " var args = a;", "'args' is already defined.")); - hints.add(createError("W041", 12, 5, " if (list.length == 0)", "Use '===' to compare with '0'.")); + 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); }