sfcgeorge / yard-contracts

YARD Plugin for Automatic Param Docs from Contracts.
MIT License
26 stars 3 forks source link

Weird output if a contracted function references instance methods via `self.anything` #8

Closed NobbZ closed 9 years ago

NobbZ commented 9 years ago

I already reported a similar issue in #6, but it turned out that this was not because of the return type, but because of something else, the use of self.anything!

Documentation for the following is generated fine:

class User
  def foo
    self.username
  end
end

While the following has the "two method" problem as descroibed above:

class User
  include Contracts

  Contract None => String
  def foo
    self.username
  end
end

So as soon I have an occurence of self.anything in the function the problem occurs. Even if I could circumvent the usage of self by renaming the parameter, I prefer to always use self for calling methods of the current object.

It does not matter if I define the method username with a contract or without or leaving it undefined/implicitely created by ActiveRecord.

edit Because I forgot to mention so far: I am using yard-contracts and contracts from rubygems.

NobbZ commented 9 years ago

Just tried with both gems from git master, problem is there too.

sfcgeorge commented 9 years ago

Aah thank you. Now I see what's happening.

yard-contracts registers a hook to be called on Contract method call. It then looks ahead to find the next method def call and documents it. Unfortunately YARD doesn't provide much info about the def call—it doesn't say whether it's a class or instance method. So currently I'm using match(/ self\./) to work out if it's a class method or not, I realise now that it's looking at the whole source of the method, not just the first line. Should be an easy fix with a stricter regex, but I don't want to make it too strict in case anyone formats their method definitions weirdly.

I'll try to fix it later today.

NobbZ commented 9 years ago

This sounds also as if you would not catch up Classmethods defined like this:

class Foo
  include Contracts
  class << self
    Contract None => String
    def bar
      'class method!'
    end
  end
end

Since I'm on my mobile right now, I can't test it by myself.

When I'm able to test this I will do and file a new issue if necessary (or someone else does faster than me).

sfcgeorge commented 9 years ago

Yep it wouldn't. It also won't work for define_method. If you can think of a better way feel free to make a pull request, but YARD + Ripper don't make it easy.

NobbZ commented 9 years ago

My knowledge about yard is only on how to use it, not on how to enhence it. But maybe the Yard-developers would be open for suggestions, so you can work together with them to enhence their interface.

waterlink commented 9 years ago

I would go here with /def self./ - will not support singleton classes, but will fix current bug. On Apr 2, 2015 3:02 PM, "Norbert Melzer" notifications@github.com wrote:

My knowledge about yard is only on how to use it, not on how to enhence it. But maybe the Yard-developers would be open for suggestions, so you can work together with them to enhence their interface.

— Reply to this email directly or view it on GitHub https://github.com/sfcgeorge/yard-contracts/issues/8#issuecomment-88888916 .

sfcgeorge commented 9 years ago

Yeah that's what I'm thinking. It also won't work if one uses the class name instead of self, but I don't know why you'd want to do that and it's against the style guide.

I think fixing singleton classes and define_method would require substantial AST parsing and I'm not sure it's worth it. I'll fix this first then suggestions welcome :)

sfcgeorge commented 9 years ago

This is now fixed with /def +self\./ and I've pushed a patch release to Rubygems. I'll close this issue as it's now fixed and maybe think about the more complex cases later.