square / cane Goto Github PK

People still email me about this repo, I want to clean it up a bit (merge some PRs, close some issues)

thx ❤️

On a 130K line project: ABC checks take about 10 seconds, doc about 1, and style about 2. I suspect the work being done is easily parrellizable.

Is this intentional?

# Bar!
class Foo
  class WorldError < StandardError; end

  # I need a comment even though I have no methods =(
  class UnusedError < StandardError; end

  def bar(the_sky_is_falling = false)
    if the_sky_is_falling
      raise WorldError.new('oh noes!')
    else
      puts 'hello world!'
    end
  end
end

cane -f foo.rb

Class definitions require explanatory comments on preceding line (1):

  foo.rb:3  WorldError

Total Violations: 1

Would be neat if it enforced New Hash Syntax.

The stack trace

uninitialized constant Tailor::FileLine
/Users/ywen/.rvm/rubies/ruby-1.9.3-p125/lib/ruby/1.9.1/rake/ext/module.rb:36:in `const_missing'
/Users/ywen/.rvm/gems/ruby-1.9.3-p125@galaxy-interactors/gems/cane-1.2.0/lib/cane/style_check.rb:38:in `<module:Cane>'
/Users/ywen/.rvm/gems/ruby-1.9.3-p125@galaxy-interactors/gems/cane-1.2.0/lib/cane/style_check.rb:5:in `<top (required)>'
/Users/ywen/.rvm/rubies/ruby-1.9.3-p125/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
/Users/ywen/.rvm/rubies/ruby-1.9.3-p125/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
/Users/ywen/.rvm/gems/ruby-1.9.3-p125@galaxy-interactors/gems/cane-1.2.0/lib/cane.rb:2:in `<top (required)>'
/Users/ywen/.rvm/rubies/ruby-1.9.3-p125/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
/Users/ywen/.rvm/rubies/ruby-1.9.3-p125/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
/Users/ywen/.rvm/gems/ruby-1.9.3-p125@galaxy-interactors/gems/cane-1.2.0/lib/cane/cli.rb:1:in `<top (required)>'
/Users/ywen/.rvm/rubies/ruby-1.9.3-p125/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
/Users/ywen/.rvm/rubies/ruby-1.9.3-p125/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
/Users/ywen/.rvm/gems/ruby-1.9.3-p125@galaxy-interactors/gems/cane-1.2.0/lib/cane/rake_task.rb:52:in `block in initialize'

I have a hash used to generate a vcard that looks like this:

def vcard_params
    {
        :first_name => first_name,
        :last_name => last_name,
        :org => organization,
        :title => title,
        :email => email,
        :phone => office,
        :mobile => mobile,
        :fax => fax,
        :website => website,
        :address => address,
        :city => city,
        :state => state,
        :zip => zip,
        :country => country
    }.delete_if{ |k,v| v.blank? }
end

And it gets an ABC score of A = 1, B = 18, C = 0. Why is the B score so high?

edit: delete_if isn't causing the problem, I removed it and the score stayed the same.

I like to use Cane to check just the file I'm currently working on - which, AFAICT, currently requires specifying the filename for three options separately:

$ cane --abc-glob $filename --style-glob $filename --doc-glob $filename

It might be nice to provide a shortcut for this - e.g. accepting individual filenames as regular (non-option) arguments:

$ cane /path/to/file_a /path/to/file_b

I'd be happy to take a crack at implementing this if it was deemed desirable.

I use the magic_encoding gem to automatically insert 'magic' UTF-8 comments into all of my Ruby files.

All of the files that have had a magic encoding line inserted are reported by cane to be documented, which they definitely are not. It looks like cane is fooled by simply having a comment on the line preceding the declaration of a class.

magic encoding line:

# -*- encoding : utf-8 -*-

E.g., I want to enforce a doc requirement for my top level directory, but would like to drop in .cane files with --no-doc in some of the subdirectories.

It'd be nice to have a global .cane file that's loaded from the home directory, similar to guard and the .gemrc file. Any thoughts on this?

Thanks so much for your tool.

Did not know enough about how to fix the documentation checks so turned them off (--no-doc) for now.

Can you please add something to the README or the tool output to explain what to do (template, example, URL)?

Thanks

I realize that there are also many other ways to alert on newline at end of file, but I feel like it would fit into cane's purpose pretty well, since there are also other whitespace/tab validations.

Most of the good metric tools out there have some way to generate HTML reports. It'd be nice if cane did that.

The regex is too liberal. It finds the following class as undocumented:

module FaradayMiddleware
  # Public: Parse a Transfer-Encoding: Chunked response to just the original data
  class Chunked < FaradayMiddleware::ResponseMiddleware
    TRANSFER_ENCODING = 'transfer-encoding'.freeze
    # ...
  end
end

As it stands, when I put .cane files in subdirectories, those preferences are reflected for files that fall in or under the same branch of directory hierarchy when I run cane from the command line. However, Cane::RakeTask only picks up preferences established in a .cane file in the root folder rather than subfolders.

If you want to use cane for CI but have certain checks you want to ignore for certain folders, that doesn't seem possible right now with a single rake task.

The documentation check appears to only check classes, not modules. It'd be nice if it checked modules, too, since I use them so frequently.

Parser: https://github.com/whitequark/parser/

Because cane depends on Ripper, it can't be run on Rubinius. Parser claims to be harder, better, faster, and stronger than the built-in Ripper API.

Might be useful for others.
https://github.com/rainerborene/vim-cane

I finally made a redacted version of the code that is showing this error. It is at https://github.com/compwron/stacklevel_cane_fail

This issue was previously in #90

cane checks currently fail on Ruby 3 because of this call to File.exists?:

This now gives a NoMethodError and suggests File.exist?.

Typical Ruby to delete the method with the correct grammar, if you ask me. :(

@xaviershay Can you take a look at this and let me know your thoughts? https://github.com/justincampbell/guard-cane

Copy how rspec allows custom formatters. For bonus points, figure out a way to allow those custom formatters to also support CLI options i.e. cane --check MyCheck --help shows --mycheck-some-var in the output. (This might be too ugly a thing to be done with OptionParser.)

I have a file called readme.md and it is failing the documentation check because its not all-caps. GitHub doesn't seem to mind, is there any reason cane does?

The names of the options used in the Rake task seem to not reflect their usage. For instance, this sets the quality check to not check ABC metrics. When it seems the opposite would be true.

Cane::RakeTask.new(:quality) do |cane|
  cane.no_abc = false
end

I noticed while testing cane that the Cane::AbcCheck#process_ast method does not seem to measure the complexity of class methods.

when I execute:

bundle exec cane --abc-glob '{lib,spec}/*/.rb' --abc-max 15

this error appears:

gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/style_check.rb:28:in `violations_for_line': invalid byte sequence in UTF-8 (ArgumentError)
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/style_check.rb:14:in `block (2 levels) in violations'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/style_check.rb:42:in `each_line'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/style_check.rb:42:in `each'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/style_check.rb:42:in `map'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/style_check.rb:42:in `with_index'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/style_check.rb:42:in `map_lines'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/style_check.rb:13:in `block in violations'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/style_check.rb:12:in `map'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/style_check.rb:12:in `violations'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane.rb:41:in `block in violations'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane.rb:39:in `each'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane.rb:39:in `map'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane.rb:39:in `violations'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane.rb:29:in `run'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane.rb:9:in `run'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/lib/cane/cli.rb:13:in `run'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/gems/cane-1.4.0/bin/cane:5:in `<top (required)>'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/bin/cane:19:in `load'
    from /home/nicolas/.rvm/gems/ruby-1.9.2-p290/bin/cane:19:in `<main>'

any help would be appreciated.

RUBYGEMS VERSION: 1.8.10
RUBY VERSION: 1.9.2 (2011-07-09 patchlevel 290) [i686-linux]

thanks!

It's not uncommon to re-open a class in another file, but Cane considers these doc violations if no documentation is included (even if the class is documented elsewhere). Cane could track documented classes (and delete previously-reported violations as it goes if a class is documented in a later file), but I'm not sure if that's the best way to handle things.

RubyGems.org doesn't report a license for your gem. This is because it is not specified in the gemspec of your last release.

via e.g.

spec.license = 'MIT'
# or
spec.licenses = ['MIT', 'GPL-2']

Including a license in your gemspec is an easy way for rubygems.org and other tools to check how your gem is licensed. As you can imagine, scanning your repository for a LICENSE file or parsing the README, and then attempting to identify the license or licenses is much more difficult and more error prone. So, even for projects that already specify a license, including a license in your gemspec is a good practice. See, for example, how rubygems.org uses the gemspec to display the rails gem license.

There is even a License Finder gem to help companies/individuals ensure all gems they use meet their licensing needs. This tool depends on license information being available in the gemspec. This is an important enough issue that even Bundler now generates gems with a default 'MIT' license.

I hope you'll consider specifying a license in your gemspec. If not, please just close the issue with a nice message. In either case, I'll follow up. Thanks for your time!

Appendix:

If you need help choosing a license (sorry, I haven't checked your readme or looked for a license file), GitHub has created a license picker tool. Code without a license specified defaults to 'All rights reserved'-- denying others all rights to use of the code.
Here's a list of the license names I've found and their frequencies

p.s. In case you're wondering how I found you and why I made this issue, it's because I'm collecting stats on gems (I was originally looking for download data) and decided to collect license metadata,too, and make issues for gemspecs not specifying a license as a public service :). See the previous link or my blog post about this project for more information.

I prefer to place a limit on the size of my classes and methods. I can do this in tailor, but it is incredibly slow compared to cane, plus, I'd prefer to have just one command to run, outside of rake to check the quality on a pre-commit hook.

Besides forking/monkey patching... is there anything built in?

I've got a class defined like so:

module Packrat
  CollectionContainer = Struct.new(:collection_occurrence) do
    def metadata
      # some code that violates my ABC threshold
    end
  end
end

The output from the ABC check is:

Methods exceeded maximum allowed ABC complexity (1):

  lib/packrat/structs.rb  Packrat > metadata  12

Notice that the method is listed as Packrat > metadata rather than Packrat > CollectionContainer > metadata. I can easily see why: I'm defining the method in the block passed to Struct.new, and cane doesn't realize that it's being defined on the CollectionContainer class since it's not the normal class CollectionContainer ... end style class definition.

It'd be reasonable to simply say "this isn't supported by cane", but if it wasn't too difficult, it'd be nice. I prefer defining struct classes using this block form over class CollectionContainer < Struct.new because the latter form subclasses a subclass of Struct (since Struct.new returns a Struct subclass) and therefore has an extra class in the ancestor chain, which can slow down method dispatch a bit by giving ruby an extra stop on the list of ancestors it traverses when dispatching methods.

If something is done to handle this case, it can be applied to defining classes using Class.new { ... } as well, since it's basically the same issue.

Assuming I've understood the point at which cane hooks into the testing lifecycle, it would be nice if we could drop a require 'cane' statement (or similar) into the Rspec spec_helper.rb, and have cane automatically execute as part of the Rspec suite without having to invoke a separate command.

09:37:53 $ cane
/Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane/abc_check.rb:30:in `process_ast': undefined method `[]' for nil:NilClass (NoMethodError)
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane/abc_check.rb:22:in `find_violations'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane/abc_check.rb:13:in `block in violations'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane/abc_check.rb:12:in `map'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane/abc_check.rb:12:in `violations'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane.rb:41:in `block in violations'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane.rb:39:in `each'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane.rb:39:in `map'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane.rb:39:in `violations'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane.rb:29:in `run'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane.rb:9:in `run'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/lib/cane/cli.rb:13:in `run'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/lib/ruby/gems/1.9.1/gems/cane-1.0.0/bin/cane:9:in `<top (required)>'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/bin/cane:19:in `load'
    from /Users/sobrinho/.rbenv/versions/1.9.3-p0/bin/cane:19:in `<main>'

What you need to debug the reason? :)

Would be helpful if cane had a -r option to scan files in a directory recursively.

Would be even better if this were on by default, so that cane . did something a bit more useful than:

$ cane .
(no output)

for a directory containing hundreds of unlinted Ruby files.

On a new project I naively tried to do this:

desc "Run cane to check quality metrics"
Cane::RakeTask.new(:cane) do |cane|
  cane.add_threshold 'coverage/covered_percent.txt', :==, 100 
end

...and cane didn't complain when the coverage percent was not 100%.

Looking at the code, I can see now that it only supports >=, but this was counterintuitive to me: why accept an operator argument if you're only going to support one operator? Worse, it did not give me a warning or an error that my threshold check wasn't being run at all.

The value "class" raises exception in a multi-line definition of an array using %w.

HTML_ATTRS = %w[
  class
  style
  ...
]

raises undefined method `[]' for nil:NilClass (NoMethodError) in line

In method find_violations the condition

class_definition?(line) && !comment?(last_line)

is not specific enough. Something like

class_definition?(line) && !comment?(last_line) && !array_context?

would be great.

A workaround is, to make non-class-definition-lines not match

/^\s*class\s+/

For the example above, this means:

HTML_ATTRS = %w[ class
  style
  ...
]

I found that --max-violations is being ignored whether I give is as a cli input or in a .cane file, for a single file (specified with -f) or for an entire project.

I'm simply running

cane --max-violations 222

Let me know if you need more details or an example file - it's just that the option is being ignored on anything I try.

wrong number of arguments (2 for 1)
/Users/jsmith/.rvm/gems/ruby-1.9.3-p448@myapp/gems/cane-2.6.0/lib/cane/json_formatter.rb:8:in `initialize'

It looks like when output colorization was added in 652c2f8
an options hash parameter was added to the call to initialize the formatter, but it wasn't added to the initializer for JsonFormatter.

Pull Request forthcoming

Running against cane 2.5.2

on osx snowleopard, cruby 2.0.0-p195 installed via rvm, run via metric_fu 4.4.0

~/.rvm/gems/ruby-2.0.0-p195@global/gems/cane-2.5.2/lib/cane/style_check.rb:57:in `violations_for_line': invalid byte sequence in UTF-8 (ArgumentError)
        from ~/.rvm/gems/ruby-2.0.0-p195@global/gems/cane-2.5.2/lib/cane/style_check.rb:40:in `block (2 levels) in violations'

It would appear this was fixed by cane 2.6.0, but I didn't see a ticket for that, so am creating an issue to document it.

My guess it was the Bugfix: better handling of invalid strings

The following should not leak an OptionParser error. Should display --help the same way as when an unknown option is specified.

> cane -abc-max                                                      
gems/cane-2.0.0/lib/cane/cli/spec.rb:46:in `parse': ambiguous option: -abc-max (OptionParser::AmbiguousOption)