[Fix #7669] Add Bundler/GemVersion cop #9727
Conversation
| - 'required' | ||
| - 'prohibited' | ||
| Include: | ||
| - '**/Gemfile' |
There was a problem hiding this comment.
I saw that other Bundler cops also include *.gemfile and gems.rb. I've personally never worked with this format of gem file, so I was unsure what exactly to test.
There was a problem hiding this comment.
The format is exactly the same, it's just a different filename.
timlkelly
force-pushed
the
gem-version-declaration
branch
from
62071c0
to
5951011
Compare
timlkelly
force-pushed
the
gem-version-declaration
branch
from
5951011
to
74b5bfb
Compare
|
The CircleCI builds do not provide a link to check their status. Is there a way I can check on their status? They seem to be queued but haven't been processed as far as I can tell. |
| class GemVersionDeclaration < Base | ||
| include ConfigurableEnforcedStyle | ||
|
|
||
| REQUIRED_MSG = 'Gem version declaration is required.' |
There was a problem hiding this comment.
declaration -> specification.
config/default.yml
Outdated
| EnforcedStyle: 'required' | ||
| SupportedStyles: | ||
| - 'required' | ||
| - 'prohibited' |
There was a problem hiding this comment.
I think we've typically used "forbidden" in the codebase, but you'll have to check this.
There was a problem hiding this comment.
Forbidden definitely seems to be more common than prohibited, good to know!
config/default.yml
Outdated
| - 'prohibited' | ||
| Include: | ||
| - '**/Gemfile' | ||
| IgnoredGems: [] |
There was a problem hiding this comment.
Lately I've favoured more names like "AllowedGems" instead of "IgnoredGems". Sadly, our use of both is a bit inconsistent.
|
I'd probably name this just "GemVersion" or something like this. Overall, I think that's a nice cop to have. |
|
|
||
| REQUIRED_MSG = 'Gem version declaration is required.' | ||
| PROHIBITED_MSG = 'Gem version declaration is prohibited.' | ||
| VERSION_DECLARATION_REGEX = /^[~<>=]*\s?[0-9.]+/.freeze |
There was a problem hiding this comment.
| VERSION_DECLARATION_REGEX = /^[~<>=]*\s?[0-9.]+/.freeze | |
| VERSION_DECLARATION_REGEX = /^\s*[~<>=]*\s*[0-9.]+/.freeze |
Multiple spaces are valid in bundler before and after the operator. We also could be more specific about the format here, but it probably doesn't really matter.
There was a problem hiding this comment.
I've added the suggested change.
We also could be more specific about the format here
Do you mind detailing what you mean by this? Or is it the change you provided?
There was a problem hiding this comment.
What I mean is the Regexp will match ...... or >>>>> 7 or other random strings that aren’t version specifications, but again it probably doesn't really matter.
It could be more strict with something like ^\s*(?:~>|[<>]?=?)?\s*[0-9]+(\.[0-9]+)*
| # @!method gem_declaration?(node) | ||
| def_node_matcher :gem_declaration?, '(send nil? :gem str ...)' |
There was a problem hiding this comment.
This method is in Bundler::GemComment as well, it might be useful to extract them to a shared mixin.
There was a problem hiding this comment.
Yup, definitely. I did copy this from Bundler::GemComment to get it working originally. But I completely agree that a mixin will DRY this up.
timlkelly
force-pushed
the
gem-version-declaration
branch
from
2eee6b1
to
90db0f7
Compare
bbatsov
changed the title
|
Great work! Thanks! |
Bundler/GemVersionenforces that gem version declarations are either required or prohibited in theGemfile.Bundler/GemVersionis disabled by default.AllowedGemsconfiguration option.I originally wrote this cop for a private repository and then added more configuration options after seeing issue #7669.
The AST checks and regex caught everything I could throw at it, but I realize that is probably limited compared to the larger Ruby ecosystem as a whole. I would appreciate any feedback you may have on those checks.
References:
#9720
#7669
Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).bundle exec rake default. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.mdif the new code introduces user-observable changes. See changelog entry format for details.