Skip to content

Loading…

Withcs #966

Closed
wants to merge 11 commits into from

3 participants

@levsa

Included the checkstyle reporter in the bundle so that it can be used in rhino environment. Also updating the gradle-js-plugin so that jshint can be used to generate checkstyle reports to be parsed by jenkins after builds.

Made minor change to the checkstyle reporter to be able to print instead of using process.stdout.

Had to create a fake console to handle error prints from the event package about event listener memory leaks, because listeners doesn't seem to be removed when too many files are given as input to jshint. That issue needs to be addressed by someone who knows how.

@guyzmo

Had to create a fake console to handle error prints from the event package about event listener memory leaks, because listeners doesn't seem to be removed when too many files are given as input to jshint. That issue needs to be addressed by someone who knows how.

you should create an issue about that

@valueof
JSHint member

you should create an issue about that

There's already GH-931.

@valueof
JSHint member

Thanks but I'd rather just add support for any reporter, similar to what we have in a Node version.

@valueof valueof closed this
@levsa

It is easy (easier) to add support for all available reporters given this patch.

@valueof
JSHint member

Well, for starters this patch replaces console-browserify we use with a hand-rolled shim. It also bumps package version which is a no-go (I bump versions as a last commit of a tagged release).

@levsa

Oops, sorry about the version. The purpose of this patch was to generate a new jshint-rhino package to use in "gradle-js-plugin", instead of patching the one there. But I ended up with patching that one anyway (jshint-rhino-r12) because of performance issues with this last one.

Yes, the console-browserify issue needs to be resolved. I don't know enough about the environment for jshint to understand the problem. If someone could help out with a solution, I can switch back to console-browserify.

For now, my patches in gradle-js-plugin works fine for us. But for future versions, it would be better if the reporter support was in place in the generated rhino package of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 26, 2013
  1. @levsa
  2. @levsa
  3. @levsa
  4. @levsa
  5. @levsa
  6. @levsa

    Unified checkstyle and non-checkstyle bundle. Add option to rhino run…

    levsa committed
    …ner to enable checkstyle reporter
  7. @levsa
  8. @levsa

    Adding checkstyle report generation.

    levsa committed
    Add fake console to handle multiple files
    
    Revert console-browserify, doesn't work because of window not defined.
    
    Patch checkstyle reporter to work in rhino environment.
    
    Add checkstyle reporter to bundle. Add option to rhino runner to enable checkstyle reporter.
    
    Improve fake console to avoid error output in stdout when running in rhino.
  9. @levsa
  10. @levsa
  11. @levsa

    Remove unused verbose flag

    levsa committed
Showing with 69 additions and 25 deletions.
  1. +19 −1 make.js
  2. +2 −3 package.json
  3. +42 −13 src/platforms/rhino.js
  4. +6 −1 src/reporters/checkstyle.js
  5. +0 −7 src/stable/jshint.js
View
20 make.js
@@ -112,8 +112,25 @@ target.build = function () {
bundle.append("JSHINT = require('/src/stable/jshint.js').JSHINT;");
+ var fakeConsole = "if (typeof console === \"undefined\") {" + "\n" +
+ " var console = {" + "\n" +
+ " error: function (txt) {" + "\n" +
+ " if (typeof Packages !== \"undefined\") { // rhino" + "\n" +
+ " java.lang.System.err.println('CONSOLE.error: ' + txt);" + "\n" +
+ " }" + "\n" +
+ " }," + "\n" +
+ " trace: function () {" + "\n" +
+ " }" + "\n" +
+ " };" + "\n" +
+ "}" + "\n";
+
+ bundle.addEntry("./src/reporters/checkstyle.js");
+ bundle.append("checkstyleReporter = require('./src/reporters/checkstyle.js').reporter;");
+
[ "// " + pkg.version,
"var JSHINT;",
+ "var checkstyleReporter;",
+ fakeConsole,
bundle.bundle()
].join("\n").to("./dist/jshint-" + pkg.version + ".js");
@@ -126,4 +143,5 @@ target.build = function () {
exec("chmod +x dist/jshint-rhino-" + pkg.version + ".js");
cli.ok("Rhino");
echo("\n");
-};
+
+};
View
5 package.json
@@ -1,6 +1,6 @@
{
"name": "jshint",
- "version": "1.1.0",
+ "version": "1.1.1",
"homepage": "http://jshint.com/",
"description": "Static analysis tool for JavaScript",
@@ -36,8 +36,7 @@
"browserify": "1.16.1",
"coveraje": "0.2.x",
"nodeunit": "0.7.x",
- "sinon": "1.6.x",
- "console-browserify": "0.1.x"
+ "sinon": "1.6.x"
},
"preferGlobal": true
View
55 src/platforms/rhino.js
@@ -1,17 +1,27 @@
/*jshint boss: true, rhino: true, unused: true, undef: true, white: true, quotmark: double */
/*global JSHINT */
+/*global checkstyleReporter */
(function (args) {
"use strict";
-
+
var filenames = [];
- var optstr; // arg1=val1,arg2=val2,...
+ var reporter; // only "checkstyle" is recognized
+ var optstr; // arg1=val1,arg2=val2,... or reporter=<reporter>
var predef; // global1=true,global2,global3,...
var opts = {};
var retval = 0;
-
+ var results = [];
+ var data = [];
+ var lintData;
+
args.forEach(function (arg) {
if (arg.indexOf("=") > -1) {
+ // Check first for reporter option
+ if (arg.split("=")[0] === "reporter") {
+ reporter = arg.split("=")[1];
+ return;
+ }
if (!optstr) {
// First time it's the options.
optstr = arg;
@@ -64,23 +74,42 @@
});
}
- filenames.forEach(function (name) {
- var input = readFile(name);
-
+ filenames.forEach(function (file) {
+ var input = readFile(file);
+
if (!input) {
- print("jshint: Couldn't open file " + name);
+ print("jshint: Couldn't open file " + file);
quit(1);
}
-
+
if (!JSHINT(input, opts)) {
- for (var i = 0, err; err = JSHINT.errors[i]; i += 1) {
- print(err.reason + " (" + name + ":" + err.line + ":" + err.character + ")");
- print("> " + (err.evidence || "").replace(/^\s*(\S*(\s+\S+)*)\s*$/, "$1"));
- print("");
- }
+ JSHINT.errors.forEach(function (err) {
+ if (err) {
+ results.push({ file: file, error: err });
+ }
+ });
retval = 1;
}
+
+ lintData = JSHINT.data();
+
+ if (lintData) {
+ lintData.file = file;
+ data.push(lintData);
+ }
});
+
+ if (reporter === "checkstyle" && typeof checkstyleReporter !== "undefined") {
+ checkstyleReporter(results, data);
+ } else {
+ for (var i = 0; i < results.length; i += 1) {
+ var file = results[i].file;
+ var err = results[i].error;
+ print(err.reason + " (" + file + ":" + err.line + ":" + err.character + ")");
+ print("> " + (err.evidence || "").replace(/^\s*(\S*(\s+\S+)*)\s*$/, "$1"));
+ print("");
+ }
+ }
quit(retval);
}(arguments));
View
7 src/reporters/checkstyle.js
@@ -1,5 +1,6 @@
// Author: Boy Baukema
// http://github.com/relaxnow
+/*global print*/
module.exports =
{
reporter: function (results, data)
@@ -102,6 +103,10 @@ module.exports =
out.push("</checkstyle>");
- process.stdout.write(out.join("\n") + "\n");
+ if (typeof process !== "undefined" && process.stdout) {
+ process.stdout.write(out.join("\n") + "\n");
+ } else {
+ print(out.join("\n") + "\n");
+ }
}
};
View
7 src/stable/jshint.js
@@ -29,8 +29,6 @@
*/
/*jshint quotmark:double */
-/*global console:true */
-/*exported console */
var _ = require("underscore");
var events = require("events");
@@ -41,11 +39,6 @@ var reg = require("./reg.js");
var state = require("./state.js").state;
var style = require("./style.js");
-// We need this module here because environments such as IE and Rhino
-// don't necessarilly expose the 'console' API and browserify uses
-// it to log things. It's a sad state of affair, really.
-var console = require("console-browserify");
-
// We build the application inside a function so that we produce only a singleton
// variable. That function will be invoked immediately, and its return value is
// the JSHINT function itself.
Something went wrong with that request. Please try again.