Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positives on Style:Documentation #751

Closed
claco opened this issue Jan 21, 2014 · 7 comments
Closed

False positives on Style:Documentation #751

claco opened this issue Jan 21, 2014 · 7 comments
Assignees
Labels

Comments

@claco
Copy link
Contributor

claco commented Jan 21, 2014

The Style::Documentation cop seems to flag seemingly similar ruby files differently based on how it determines what is and isn't documentation for a class.

For example, this file raises a documentation cop violation:

# encoding: UTF-8
#
# Cookbook Name:: openstack-compute
# Recipe:: compute
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

require 'uri'

class ::Chef::Recipe
  include ::Openstack
end
$ rubocop recipes/compute.rb
Inspecting 1 file
C

Offences:

test.rb:21:1: C: Missing top-level class documentation comment.
class ::Chef::Recipe
^^^^^

Not shocking. This can be mitigated with:

 class ::Chef::Recipe # rubocop:disable Documentation
  include ::Openstack
end

However, once I hit a cookbook with e recipe that did not have a require statement, I not longer get the Documentation cop violation, but I expected to:

# encoding: UTF-8
#
# Cookbook Name:: openstack-compute
# Recipe:: compute
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

class ::Chef::Recipe
  include ::Openstack
end

yields:

$ rubocop recipes/compute.rb
Inspecting 1 file
.

1 file inspected, no offences detected

This seems like a bug. The comments at the top of the file are not documentation for class ::Chef::Recipe. The trick is teaching rubocop that is true. :-)

I don't write a log of RDoc, but when I do Yard docs, I've always put the docs on the line directly above the thing being documented (no space between the two).

Thoughts?

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 22, 2014

The parser attaches comment nodes to the code node which follows them. If there is something between the comment and the class/module node (say a require) it will not be picked up as class/module documentation comment. A file documentation/licence header is not the same as class documentation comment IMO, so I wouldn't consider this a bug.

@claco
Copy link
Contributor Author

claco commented Jan 22, 2014

"A file documentation/licence header is not the same as class documentation comment IMO, so I wouldn't consider this a bug."

That's exactly the bug. The license header is not class documentation, yet in the case of the second example, the Documentation cop thinks it is. Just because there is a comment above a class/method with a blank line between the two, doesn't mean it's documentation. I would expect rubocop to flag this class as having no documentation:

...
# See the License for the specific language governing permissions and
# limitations under the License.
#

class ::Chef::Recipe
  include ::Openstack
end

To not be in violation of a Documentation cop, I would expect this to be the correct format:

# See the License for the specific language governing permissions and
# limitations under the License.
#

# class documentation here
class ::Chef::Recipe
  include ::Openstack
end

@jonas054
Copy link
Collaborator

I agree with @claco. I think it is a bug that the file documentation count as class documentation is this case.

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 31, 2014

@jonas054 Indeed. Will you volunteer to have a look at it?

@jonas054
Copy link
Collaborator

Yes!

@ghost ghost assigned jonas054 Jan 31, 2014
bbatsov added a commit that referenced this issue Jan 31, 2014
[Fix #751] Let Documentation cop require class comment to be adjacent
@claco
Copy link
Contributor Author

claco commented Jan 31, 2014

Nice!

@jdickey
Copy link

jdickey commented Apr 15, 2015

Real head-scratcher there; if anyone has pointers, I'm all ears. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants