Wool Status Update: 12/14/2010

Posted by -

Today, I've pushed significant work on Wool. It's still very rough around the edges, but I've got it doing the following new, cool, and somewhat-advanced analyses:

Using a semicolon to separate statements

If you'd like to warn against using a semicolon to separate statements on a single line, you can turn this warning on. However, due to personal preference, if you do this:

    class MyError < StandardError; end

it won't complain. This, though:

    puts x; puts y

will be an error. Wool is capable of fixing this error while maintaining indentation levels, up to arbitrary number of violating semicolons:

    # some comment
      p x; p y; p z; p 10

becomes

    # some comment
      p x
      p y
      p z
      p 10

Cool, eh? I think I have this one nailed down to 100% accuracy, too.

Whitespace Tomfoolery

Two separate ones here, extra blank lines and trailing whitespace. Both of these are significant mainly because when you violate them, you hurt signal-to-noise in VCS diffs. Committing 2 rows deleted and added with just trailing whitespace differences is just a shame.

Extra Blank Lines

Ever leave blank lines at the end of your file? This warning tells you how many lines you left at the end, and fixes them if you wish.

Trailing Whitespace

Extra spaces and tabs at the end of a line fall in this category, and can be fixed without disturbing the source code.

Operator Spacing

Misaligned Indentation

This is another one that needs a bit of an overhaul, but it works pretty well so far. This warning checks how well you indent, namely, that you indent at consistent multiples, and that you unindent evenly. However, it doesn't yet take into account line continuation, where you should then indent relevant to the previous line (and not via the multiplication pattern). So it can detect and correct these mistakes:

    if x
      if y
          if z
            p x
          end
        end
        end

becoming

    if x
      if y
        if z
          p x
        end
      end
    end

Cool eh? It does make some mistakes though, and there are definite examples in the wool code itself that this will mess up on.

Line Length

Line Length is a fun one!

The line length warnings let you specify multiple levels of severity for different violations, such as a level 2 warning for being between 80-89 chars and a 4 for 90+. Detecting this is extremely simple. Also, it considers whitespace stripping when calculating a line's minimum possible length.

The really fun part is fixing it. Because Ruby code is sensitive to newlines, we can't just break it up into neat 80-char lines on the whitespace. So there's a few quick heuristics currently in play that I think are significant to brag about.

Comment Rearranging

If a line is over the limit, but it has a comment at the end of it, we know we can take the comment and put it on the line above. It really belongs there anyway if it's going off the edge – the line it's describing is long enough to go near the edge, so it's probably quite complex. Its comment should have its own line.

This heuristic however could create a new line that's over the limit, if the comment is comically long. So we also break the comment into correctly-sized lines, all indented to match the original line.

An example, if the max line length is 20:

    puts x * y * z / a # this computes a significant equation involving math

you'll get back:

    # this computes a
    # significant
    # equation involving
    # math
    # puts x * y * z / a

which still has no line length violations! It naturally smooths out at longer line lengths.

Guard rearranging

Here's one that bugs me. When I write a conditional Exception raise, I think of it this way:

    raise FooBarError.new('Steve forgot to write the Foo bar library') unless steve.finished?

but that's usually way over the line length limit. In truth, I should check it in like this:

    unless steve.finished?
      aise FooBarError.new('Steve forgot to write the Foo bar library')
    end

And if you use Wool now, you can leave code in the first form and it will transform to the second form! In fact, it works for multiple guards, too:

    raise FooBarError.new('Steve forgot to write the Foo bar library') unless steve.finished?(schedule) if foo_bar.missing?(search_results) unless working?(test_data, &test_proc)

wool magically transforms into:

    unless working?(test_data, &test_proc)
      if foo_bar.missing?(search_results)
        unless steve.finished?(schedule)
          raise FooBarError.new('Steve forgot to write the Foo bar library')
        end
      end
    end

Which is clean and better. It actually picks up on-the-line cases I missed on a somewhat regular basis!

That's all for line length shortening. I know there's a lot more to do with continued lines (ending a line with a + token before the line-length cutoff, for example, means you know you can break the line into a new one at that point, resulting in a smaller problem and 1 valid line. Repeating this with all the safe tokens should result in a lot of progress.

Inline comment spacing

My style guide (derived mainly from the Google style guide, with a few exceptions that everyone takes because it's a wee bit off) says that inline comments must have exactly 2 spaces between the last non-whitespace character and the hash mark beginning the comment. This is easily detected, enforced, and corrected. So Wool does!

Unnecessary Double Quotes

When you use double quotes, the parser has to go and check for string interpolation and escape sequences. This is also true when you use the %Q{} syntax. Thus it is slower to use double quotes or %Q{} when you don't need those features. Wool detects this and corrects double-quotes to single quotes (unless there is an escape sequence in the text or an apostrophe) and %Q{} to %q{}. It uses the raw AST, so it isn't error-prone like the regex-based passes.

rescue Exception

is bad! If you're rescuing Exception, you're rescuing a lot of things. You're covering up syntax errors. Just about the only thing you don't rescue is SystemExit. Almost always, you should be rescueing StandardError. Wool currently can detect this based on the AST in all cases, but it cannot auto-correct them yet.

Up Next: Immediate

Next, I'm going to start defining the API for constant-ness, with a tag on the AST that I'll make a pass to infer. I'll also be adding

    default_attr_accessor :pure, false

to Method and Proc, so methods can be annotated as Pure. Thus, a method call on a constant object with constant arguments (if any) and a constant proc for a block (because it contains only pure calls...) is itself a constant expression. All these rules can be enumerated and inferred, and once the standard library batch of annotations is underway a lot of cool examples of success are going to come out of this. For example, Rubinius could use our constant-ness evaluator to (with a flag of course, not by default) allow constant assignment in methods to constant expressions:

    def enable_global_settings
      LOGGER = DEBUG_MODE ? StdoutLogger.new : nil
    end

Down The Line

Here's a quick brain dump of down-the-line ideas:

Stay tuned, as always, for more.

Enjoy this article? Then feel free to:

Comments