Sublime Forum

Javascript clean code reviewer plugin

#1

Hi all,

Here is a plugin I just created: cleanjs
Get it from here: github.com/captainbrosset/cleanjs (with package control)

I was working recently on cleanjs, a python tool that performs code style reviews on javascript code.
cleanjs is very much code quality oriented. Indeed, it checks for variables and functions size and naming, complexity, number of lines of code, formatting, comments, syntax, etc … There are still many things I want to review, but it’s a start
The tool was originally designed to be run from the command line using python, and since about a week also online at cleanjscode.appspot.com

Since I just discovered Sublime Text 2 (AMAZING by the way), I thought I would write a plugin for cleanjs as well.

After install, press ctrl+shift+c to start the review of any currently opened JS file.
This will open a panel with the list of info, warning and error messages, as well as highlight the corresponding lines in the code.

By the way, I wasn’t able to find an API that allowed me to add tooltips to the regions and/or info bubbles in the line number gutter. This would be great!

Thanks for Sublime and the plugin API, and tell me what you think!

0 Likes
#2

Very interesting plugin. Pretty impressive.

It is a bit heavy on stylistic opinion for me though. It seems to like to complain about comments and the way I like to name my variables. I think it might be useful in the future to add the ability to tweak the ruleset to fit a user’s preference (I think I did see that on your todo list).

Stylistic opinion is kind of why I started using jshint vs jslint. Though I allow most of the the defaults for jslint, there are some things I really just don’t buy into that jshint allows me to opt out of.

I find the approach to complexity interesting as well. It is nice to see different approaches to these kind of things.

Good job and keep up the good work.

0 Likes
#3

You can put text into the status bar at the bottom (where it tells you about the current line number). Maybe that is a workaround. sublime.status_message(“foo”) might do the trick.

0 Likes
#4

Thanks for the feedback. That’s really appreciated.
I’ve indeed planned to work on configuration capabilities.
When I do, I’ll expose this through the sublime text settings so that users can fine tune them.
Complexity-wise, what I check for the moment is rather simple, but it does show some interesting results.

0 Likes
#5

Hi all,

cleanjs 0.6 is available.
It brings a number of improvements:

  • New reviewer that checks for variable length based on their “scope” (if a variable is used only on 2 consecutive lines, it might be OK for it to have a very short name (var i type of things))

  • New reviewer that checks for similarities between comment lines that immediately precede code lines. This reviewer tries to report useless stuff like:
    // this will get the data
    this.getData();

  • New review that tries to check if a file has more than one responsibilities. It does this reading comments and checking occurrences of “if”, “but”, “or”, “and”. If the developer feels the need to use these words in a comment block, it’s most of the time a sign that the code he’s trying to explain does more than one thing.

  • New reviewer that checks for size of function argument names

  • Fixed a few bugs

  • Improved unit test coverage

Install it from here: github.com/captainbrosset/cleanjs (with package control), let me know if you find any bugs!
And beware, cleanjs will really hurt your feelings :wink:

0 Likes
#6

very nice! When you do get to the configuration options, it might be a good idea to provide a way to override the config options for individual sections of code. jslint does this with comments like this:

/*jslint evil: true*/
eval("foo"); // <-- jslint doesn't complain about this
/*jslint evil: false */
eval("foo"); // <-- this is a warning.

This would be really useful for some places where the jsclean warning about variables names aren’t valid. For example, if you have some code which is doing some canvas drawing, or coordinate manipulation, then “x” and “y” are quite descriptive enough.

/*jsclean validvars: x, y */
function drawPoint(x, y) { ... }

Perhaps unlike jslint, the options should only apply to the scope in which they were defined (so that you don’t have to turn options off and then then on again. My two cents, anyway.

0 Likes
#7

You’re right about the x,y coordinates, and in fact, I already saw the problem (reported here github.com/captainbrosset/cleanjs/issues/5).

Cleanjs checks for minimum length and meaning of variables and arguments, so allowing names like x and y will be impossible.
Indeed, one solution could be to let the user express in some ways that these variables or arguments are fine.
Perhaps special statements in the code (wrapped in comment blocks) could do the trick. The following comes to mind:

//cleanjsoff
this.code(wont,be,checked);
//cleanjson

0 Likes
#8

I’d prefer a more targeted “on and off” command. For example, if that function is stupidly long, that should still be a warning. An all-or-nothing approach isn’t helpful here.

0 Likes
#9

So if it can successfully write a review of the code… could we assume that it could also successfully modify the code one day?

0 Likes