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

Add Lint/ConstantResolution cop #8142

Merged
merged 3 commits into from Jun 20, 2020
Merged

Add Lint/ConstantResolution cop #8142

merged 3 commits into from Jun 20, 2020

Commits on Jun 20, 2020

  1. Add Style/ConstantResolution cop

    Ruby constant resolution sometimes gets things wrong.
    
    I think generally gems (that are meant for including as a library, not
    like rubocop itself) should fully qualify all constants to avoid
    stepping on any toes inadvertently. I would run this on my gems without
    using `Only`/`Ignore`
    
    Large projects will probably end up with one or two constant names that
    are problematic because of a conflict with a library or just internally
    having sensible names for directories that end up being used by e.g.
    rails as namespaces in multiple different places. I would run these with
    `Only: [The, Constant, Names, Causing Issues]`
    
    Sometimes a method refers to a constant that you want to refer to a
    different constant at different calls, e.g. the method is defined on a
    superclass and the constant differs between subclasses. (e.g. MSG in
    rubocop add_offense). I would use
    `Ignore: [The, Intentionally, Abiguous, Constant]` in these cases.
    (or `self.class::MSG` now that I've looked up how add_offense works)
    
    I've left this `Enabled: false` because I'm 50-50 on whether it should
    be Enabled by default.
    
    I didn't bother creating an autocorrect because I see there being two
    options and thought I'd get feedback.
    
    1. carelessly assume the written constant is fully qualified and just
    missing a ::` prefix. This has the advantage of probably looking correct
    more often, but also being wrong a lot.
    2. look at the current namespace and prefix that. This has the advantage
    of not necessarily changing behaviour for classes. For modules leaving
    it unchanged or carelessly assuming, or checking if the constant is
    defined in the module or not before prefixing
    3. just don't autocorrect
    robotdana committed Jun 20, 2020
    Copy the full SHA
    79d6a8e View commit details
    Browse the repository at this point in the history
  2. Move Style/ConstantResolution to Lint/

    This is a separate commit so we can easily not do it if you'd rather not
    robotdana committed Jun 20, 2020
    Copy the full SHA
    96da2c4 View commit details
    Browse the repository at this point in the history
  3. Copy the full SHA
    3db3ddd View commit details
    Browse the repository at this point in the history