Add Lint/ConstantResolution cop (#8142)

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, Ambiguous, 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

CHANGELOG.md

@@ -8,6 +8,7 @@
 * [#8113](https://github.com/rubocop-hq/rubocop/pull/8113): Let `expect_offense` templates add variable-length whitespace with `_{foo}`. ([@eugeneius][])
 * [#8148](https://github.com/rubocop-hq/rubocop/pull/8148): Support autocorrection for `Style/MultilineTernaryOperator`. ([@koic][])
 * [#8151](https://github.com/rubocop-hq/rubocop/pull/8151): Support autocorrection for `Style/NestedTernaryOperator`. ([@koic][])
+* [#8142](https://github.com/rubocop-hq/rubocop/pull/8142): Add `Lint/ConstantResolution` cop. ([@robotdana][])
 
 ### Bug fixes
 

config/default.yml

@@ -1361,6 +1361,15 @@ Lint/CircularArgumentReference:
   Enabled: true
   VersionAdded: '0.33'
 
+Lint/ConstantResolution:
+  Description: 'Check that constants are fully qualified with `::`.'
+  Enabled: false
+  VersionAdded: '0.86'
+  # Restrict this cop to only looking at certain names
+  Only: []
+  # Restrict this cop from only looking at certain names
+  Ignore: []
+
 Lint/Debugger:
   Description: 'Check for debugger calls.'
   Enabled: true

docs/modules/ROOT/pages/cops.adoc

@@ -187,6 +187,7 @@ In the following section you find all available cops:
 * xref:cops_lint.adoc#lintbigdecimalnew[Lint/BigDecimalNew]
 * xref:cops_lint.adoc#lintbooleansymbol[Lint/BooleanSymbol]
 * xref:cops_lint.adoc#lintcircularargumentreference[Lint/CircularArgumentReference]
+* xref:cops_lint.adoc#lintconstantresolution[Lint/ConstantResolution]
 * xref:cops_lint.adoc#lintdebugger[Lint/Debugger]
 * xref:cops_lint.adoc#lintdeprecatedclassmethods[Lint/DeprecatedClassMethods]
 * xref:cops_lint.adoc#lintdeprecatedopensslconstant[Lint/DeprecatedOpenSSLConstant]

docs/modules/ROOT/pages/cops_lint.adoc

@@ -310,6 +310,96 @@ def cook(dry_ingredients = self.dry_ingredients)
 end
 ----
 
+== Lint/ConstantResolution
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Disabled
+| Yes
+| No
+| 0.86
+| -
+|===
+
+Check that certain constants are fully qualified.
+
+This is not enabled by default because it would mark a lot of offenses
+unnecessarily.
+
+Generally, gems should fully qualify all constants to avoid conflicts with
+the code that uses the gem. Enable this cop without using `Only`/`Ignore`
+
+Large projects will over time end up with one or two constant names that
+are problematic because of a conflict with a library or just internally
+using the same name a namespace and a class. To avoid too many unnecessary
+offenses, Enable this cop with `Only: [The, Constant, Names, Causing, Issues]`
+
+=== Examples
+
+[source,ruby]
+----
+# By default checks every constant
+
+# bad
+User
+
+# bad
+User::Login
+
+# good
+::User
+
+# good
+::User::Login
+----
+
+==== Only: ['Login']
+
+[source,ruby]
+----
+# Restrict this cop to only being concerned about certain constants
+
+# bad
+Login
+
+# good
+::Login
+
+# good
+User::Login
+----
+
+==== Ignore: ['Login']
+
+[source,ruby]
+----
+# Restrict this cop not being concerned about certain constants
+
+# bad
+User
+
+# good
+::User::Login
+
+# good
+Login
+----
+
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| Only
+| `[]`
+| Array
+
+| Ignore
+| `[]`
+| Array
+|===
+
 == Lint/Debugger
 
 |===

lib/rubocop.rb

@@ -242,6 +242,7 @@
 require_relative 'rubocop/cop/lint/big_decimal_new'
 require_relative 'rubocop/cop/lint/boolean_symbol'
 require_relative 'rubocop/cop/lint/circular_argument_reference'
+require_relative 'rubocop/cop/lint/constant_resolution'
 require_relative 'rubocop/cop/lint/debugger'
 require_relative 'rubocop/cop/lint/deprecated_class_methods'
 require_relative 'rubocop/cop/lint/deprecated_open_ssl_constant'

lib/rubocop/cop/lint/constant_resolution.rb

@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Lint
+      # Check that certain constants are fully qualified.
+      #
+      # This is not enabled by default because it would mark a lot of offenses
+      # unnecessarily.
+      #
+      # Generally, gems should fully qualify all constants to avoid conflicts with
+      # the code that uses the gem. Enable this cop without using `Only`/`Ignore`
+      #
+      # Large projects will over time end up with one or two constant names that
+      # are problematic because of a conflict with a library or just internally
+      # using the same name a namespace and a class. To avoid too many unnecessary
+      # offenses, Enable this cop with `Only: [The, Constant, Names, Causing, Issues]`
+      #
+      # @example
+      #   # By default checks every constant
+      #
+      #   # bad
+      #   User
+      #
+      #   # bad
+      #   User::Login
+      #
+      #   # good
+      #   ::User
+      #
+      #   # good
+      #   ::User::Login
+      #
+      # @example Only: ['Login']
+      #   # Restrict this cop to only being concerned about certain constants
+      #
+      #   # bad
+      #   Login
+      #
+      #   # good
+      #   ::Login
+      #
+      #   # good
+      #   User::Login
+      #
+      # @example Ignore: ['Login']
+      #   # Restrict this cop not being concerned about certain constants
+      #
+      #   # bad
+      #   User
+      #
+      #   # good
+      #   ::User::Login
+      #
+      #   # good
+      #   Login
+      #
+      class ConstantResolution < Cop
+        MSG = 'Fully qualify this constant to avoid possibly ambiguous resolution.'
+
+        def_node_matcher :unqualified_const?, <<~PATTERN
+          (const nil? #const_name?)
+        PATTERN
+
+        def on_const(node)
+          return unless unqualified_const?(node)
+
+          add_offense(node)
+        end
+
+        private
+
+        def const_name?(name)
+          name = name.to_s
+          (allowed_names.empty? || allowed_names.include?(name)) &&
+            !ignored_names.include?(name)
+        end
+
+        def allowed_names
+          cop_config['Only']
+        end
+
+        def ignored_names
+          cop_config['Ignore']
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/lint/constant_resolution_spec.rb

@@ -0,0 +1,113 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Lint::ConstantResolution, :config do
+  it 'registers no offense when qualifying a const' do
+    expect_no_offenses(<<~RUBY)
+      ::MyConst
+    RUBY
+  end
+
+  it 'registers no offense qualifying a namespace const' do
+    expect_no_offenses(<<~RUBY)
+      ::MyConst::MY_CONST
+    RUBY
+  end
+
+  it 'registers an offense not qualifying a const' do
+    expect_offense(<<~RUBY)
+      MyConst
+      ^^^^^^^ Fully qualify this constant to avoid possibly ambiguous resolution.
+    RUBY
+  end
+
+  it 'registers an offense not qualifying a namespace const' do
+    expect_offense(<<~RUBY)
+      MyConst::MY_CONST
+      ^^^^^^^ Fully qualify this constant to avoid possibly ambiguous resolution.
+    RUBY
+  end
+
+  context 'with Only set' do
+    let(:cop_config) { { 'Only' => ['MY_CONST'] } }
+
+    it 'registers no offense when qualifying a const' do
+      expect_no_offenses(<<~RUBY)
+        ::MyConst
+      RUBY
+    end
+
+    it 'registers no offense qualifying a namespace const' do
+      expect_no_offenses(<<~RUBY)
+        ::MyConst::MY_CONST
+      RUBY
+    end
+
+    it 'registers no offense not qualifying another const' do
+      expect_no_offenses(<<~RUBY)
+        MyConst
+      RUBY
+    end
+
+    it 'registers no with a namespace const' do
+      expect_no_offenses(<<~RUBY)
+        MyConst::MY_CONST
+      RUBY
+    end
+
+    it 'registers an offense with an unqualified const' do
+      expect_offense(<<~RUBY)
+        MY_CONST
+        ^^^^^^^^ Fully qualify this constant to avoid possibly ambiguous resolution.
+      RUBY
+    end
+
+    it 'registers an offense when an unqualified namespace const' do
+      expect_offense(<<~RUBY)
+        MY_CONST::B
+        ^^^^^^^^ Fully qualify this constant to avoid possibly ambiguous resolution.
+      RUBY
+    end
+  end
+
+  context 'with Ignore set' do
+    let(:cop_config) { { 'Ignore' => ['MY_CONST'] } }
+
+    it 'registers no offense when qualifying a const' do
+      expect_no_offenses(<<~RUBY)
+        ::MyConst
+      RUBY
+    end
+
+    it 'registers no offense qualifying a namespace const' do
+      expect_no_offenses(<<~RUBY)
+        ::MyConst::MY_CONST
+      RUBY
+    end
+
+    it 'registers an offense not qualifying another const' do
+      expect_offense(<<~RUBY)
+        MyConst
+        ^^^^^^^ Fully qualify this constant to avoid possibly ambiguous resolution.
+      RUBY
+    end
+
+    it 'registers an with a namespace const' do
+      expect_offense(<<~RUBY)
+        MyConst::MY_CONST
+        ^^^^^^^ Fully qualify this constant to avoid possibly ambiguous resolution.
+      RUBY
+    end
+
+    it 'registers no offense with an unqualified const' do
+      expect_no_offenses(<<~RUBY)
+        MY_CONST
+      RUBY
+    end
+
+    it 'registers no offense when an unqualified namespace const' do
+      expect_no_offenses(<<~RUBY)
+        MY_CONST::B
+      RUBY
+    end
+  end
+end