Stop passing member name to `strong_memoize_attr`

Replace "!" and "?" with their unicode counterparts.

Example:
  key? becomes key？
  key! becomes key！

Fix existing use of `strong_memoize_attr` with member name.

Before:
  strong_memoize_attr :enabled?, :enabled

After:
  strong_memoize_attr :enabled?

app/controllers/concerns/lfs_request.rb

@@ -80,7 +80,7 @@ def render_lfs_not_found
   def lfs_download_access?
     ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code? || deploy_token_can_download_code?
   end
-  strong_memoize_attr :lfs_download_access?, :lfs_download_access
+  strong_memoize_attr :lfs_download_access?
 
   def deploy_token_can_download_code?
     deploy_token.present? &&
@@ -97,7 +97,7 @@ def lfs_upload_access?
       can?(deploy_token, :push_code, project) ||
       any_branch_allows_collaboration?
   end
-  strong_memoize_attr :lfs_upload_access?, :lfs_upload_access
+  strong_memoize_attr :lfs_upload_access?
 
   def any_branch_allows_collaboration?
     project.merge_requests_allowing_push_to_user(user).any?

app/finders/git_refs_finder.rb

@@ -49,7 +49,7 @@ def find_exact_match_index(matches, term)
   def regex_search?
     Regexp.union('^', '$', '*') === search
   end
-  strong_memoize_attr :regex_search?, :regex_search
+  strong_memoize_attr :regex_search?
 
   def unescape_regex_operators(regex_string)
     regex_string.sub('\^', '^').gsub('\*', '.*?').sub('\$', '$')

app/models/group.rb

@@ -846,7 +846,7 @@ def parent_allows_two_factor_authentication?
   def has_project_with_service_desk_enabled?
     Gitlab::ServiceDesk.supported? && all_projects.service_desk_enabled.exists?
   end
-  strong_memoize_attr :has_project_with_service_desk_enabled?, :has_project_with_service_desk_enabled
+  strong_memoize_attr :has_project_with_service_desk_enabled?
 
   def activity_path
     Gitlab::Routing.url_helpers.activity_group_path(self)

app/models/project_setting.rb

@@ -59,7 +59,7 @@ def show_diff_preview_in_email?
       !!super
     end
   end
-  strong_memoize_attr :show_diff_preview_in_email?, :show_diff_preview_in_email
+  strong_memoize_attr :show_diff_preview_in_email?
 
   private
 

doc/development/utilities.md

@@ -188,7 +188,7 @@ Refer to [`strong_memoize.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/maste
     def enabled?
       Feature.enabled?(:some_feature)
     end
-    strong_memoize_attr :enabled?, :enabled
+    strong_memoize_attr :enabled?
   end
   ```
 

ee/app/services/geo/repository_verification_secondary_service.rb

@@ -189,7 +189,7 @@ def verification_started!
     def wiki_repository_registry_synced?
       type == :wiki && wiki_repository_registry.synced?
     end
-    strong_memoize_attr :wiki_repository_registry_synced?, :wiki_repository_registry_synced
+    strong_memoize_attr :wiki_repository_registry_synced?
 
     def wiki_repository_registry
       @wiki_repository_registry ||=

ee/app/services/users/email_verification/send_custom_confirmation_instructions_service.rb

@@ -43,7 +43,7 @@ def send_instructions
       def enabled?
         self.class.enabled?(user.email)
       end
-      strong_memoize_attr :enabled?, :enabled
+      strong_memoize_attr :enabled?
 
       def self.enabled?(email)
         return false if ::Feature.enabled?(:soft_email_confirmation)

ee/lib/gitlab/auth/group_saml/identity_linker.rb

@@ -77,7 +77,7 @@ def extern_uid_update_required?
         def update_extern_uid_allowed?
           current_user.verified_email?(auth_hash.email)
         end
-        strong_memoize_attr :update_extern_uid_allowed?, :update_extern_uid_allowed
+        strong_memoize_attr :update_extern_uid_allowed?
 
         def update_extern_uid_failed?
           extern_uid_update_required? && !update_extern_uid_allowed?

lib/gitlab/ci/pipeline/logger.rb

@@ -121,7 +121,7 @@ def log?
         def enabled?
           ::Feature.enabled?(:ci_pipeline_creation_logger, project, type: :ops)
         end
-        strong_memoize_attr :enabled?, :enabled
+        strong_memoize_attr :enabled?
 
         def observations
           @observations ||= {}

lib/gitlab/ci/pipeline/seed/build.rb

@@ -53,7 +53,7 @@ def included?
               end
             end
           end
-          strong_memoize_attr :included?, :inclusion
+          strong_memoize_attr :included?
 
           def errors
             logger.instrument(:pipeline_seed_build_errors) do
@@ -261,7 +261,7 @@ def calculate_yaml_variables!
           def reuse_build_in_seed_context?
             Feature.enabled?(:ci_reuse_build_in_seed_context, @pipeline.project)
           end
-          strong_memoize_attr :reuse_build_in_seed_context?, :reuse_build_in_seed_context
+          strong_memoize_attr :reuse_build_in_seed_context?
         end
       end
     end

lib/gitlab/utils/strong_memoize.rb

@@ -23,7 +23,7 @@ module StrongMemoize
       #     def enabled?
       #       Feature.enabled?(:some_feature)
       #     end
-      #     strong_memoize_attr :enabled?, :enabled
+      #     strong_memoize_attr :enabled?
       #
       def strong_memoize(name)
         key = ivar(name)
@@ -46,20 +46,20 @@ def strong_memoize_with(name, *args)
       end
 
       def strong_memoized?(name)
-        instance_variable_defined?(ivar(name))
+        key = ivar(StrongMemoize.normalize_key(name))
+        instance_variable_defined?(key)
       end
 
       def clear_memoization(name)
-        key = ivar(name)
+        key = ivar(StrongMemoize.normalize_key(name))
         remove_instance_variable(key) if instance_variable_defined?(key)
       end
 
       module StrongMemoizeClassMethods
-        def strong_memoize_attr(method_name, member_name = nil)
-          member_name ||= method_name
+        def strong_memoize_attr(method_name)
+          member_name = StrongMemoize.normalize_key(method_name)
 
-          StrongMemoize.send( # rubocop:disable GitlabSecurity/PublicSend
-            :do_strong_memoize, self, method_name, member_name)
+          StrongMemoize.send(:do_strong_memoize, self, method_name, member_name) # rubocop:disable GitlabSecurity/PublicSend
         end
       end
 
@@ -83,7 +83,14 @@ def ivar(name)
         end
       end
 
-      class <<self
+      class << self
+        def normalize_key(key)
+          return key unless key.end_with?('!', '?')
+
+          # Replace invalid chars like `!` and `?` with allowed Unicode codeparts.
+          key.to_s.tr('!?', "\uFF01\uFF1F")
+        end
+
         private
 
         def do_strong_memoize(klass, method_name, member_name)

rubocop/cop/gitlab/strong_memoize_attr.rb

@@ -39,7 +39,7 @@ class StrongMemoizeAttr < RuboCop::Cop::Base
         def_node_matcher :strong_memoize?, <<~PATTERN
           (block
             $(send nil? :strong_memoize
-              (sym $_)
+              (sym _)
             )
             (args)
             $_
@@ -47,21 +47,20 @@ class StrongMemoizeAttr < RuboCop::Cop::Base
         PATTERN
 
         def on_block(node)
-          send_node, attr_name, body = strong_memoize?(node)
+          send_node, body = strong_memoize?(node)
           return unless send_node
 
-          corrector = autocorrect_pure_definitions(node.parent, attr_name, body) if node.parent.def_type?
+          corrector = autocorrect_pure_definitions(node.parent, body) if node.parent.def_type?
 
           add_offense(send_node, &corrector)
         end
 
         private
 
-        def autocorrect_pure_definitions(def_node, attr_name, body)
+        def autocorrect_pure_definitions(def_node, body)
           proc do |corrector|
             method_name = def_node.method_name
-            attr_suffix = ", :#{attr_name}" if attr_name != method_name
-            replacement = "\n#{indent(def_node)}strong_memoize_attr :#{method_name}#{attr_suffix}"
+            replacement = "\n#{indent(def_node)}strong_memoize_attr :#{method_name}"
 
             corrector.insert_after(def_node, replacement)
             corrector.replace(def_node.body, body.source)

spec/lib/gitlab/utils/strong_memoize_spec.rb

@@ -2,12 +2,13 @@
 
 require 'fast_spec_helper'
 require 'rspec-benchmark'
+require 'rspec-parameterized'
 
 RSpec.configure do |config|
   config.include RSpec::Benchmark::Matchers
 end
 
-RSpec.describe Gitlab::Utils::StrongMemoize do
+RSpec.describe Gitlab::Utils::StrongMemoize, feature_category: :not_owned do
   let(:klass) do
     strong_memoize_class = described_class
 
@@ -35,15 +36,10 @@ def method_name_attr
       end
       strong_memoize_attr :method_name_attr
 
-      def different_method_name_attr
+      def enabled?
         trace << value
         value
       end
-      strong_memoize_attr :different_method_name_attr, :different_member_name_attr
-
-      def enabled?
-        true
-      end
       strong_memoize_attr :enabled?
 
       def method_name_with_args(*args)
@@ -80,6 +76,8 @@ def public_method; end
   subject(:object) { klass.new(value) }
 
   shared_examples 'caching the value' do
+    let(:member_name) { described_class.normalize_key(method_name) }
+
     it 'only calls the block once' do
       value0 = object.send(method_name)
       value1 = object.send(method_name)
@@ -103,7 +101,6 @@ def public_method; end
       context "with value #{value}" do
         let(:value) { value }
         let(:method_name) { :method_name }
-        let(:member_name) { :method_name }
 
         it_behaves_like 'caching the value'
 
@@ -176,31 +173,44 @@ def public_method; end
   end
 
   describe '#strong_memoized?' do
-    let(:value) { :anything }
+    shared_examples 'memoization check' do |method_name|
+      context "for #{method_name}" do
+        let(:value) { :anything }
 
-    subject { object.strong_memoized?(:method_name) }
+        subject { object.strong_memoized?(method_name) }
 
-    it 'returns false if the value is uncached' do
-      is_expected.to be(false)
-    end
+        it 'returns false if the value is uncached' do
+          is_expected.to be(false)
+        end
 
-    it 'returns true if the value is cached' do
-      object.method_name
+        it 'returns true if the value is cached' do
+          object.public_send(method_name)
 
-      is_expected.to be(true)
+          is_expected.to be(true)
+        end
+      end
     end
+
+    it_behaves_like 'memoization check', :method_name
+    it_behaves_like 'memoization check', :enabled?
   end
 
   describe '#clear_memoization' do
-    let(:value) { 'mepmep' }
+    shared_examples 'clearing memoization' do |method_name|
+      let(:member_name) { described_class.normalize_key(method_name) }
+      let(:value) { 'mepmep' }
 
-    it 'removes the instance variable' do
-      object.method_name
+      it 'removes the instance variable' do
+        object.public_send(method_name)
 
-      object.clear_memoization(:method_name)
+        object.clear_memoization(method_name)
 
-      expect(object.instance_variable_defined?(:@method_name)).to be(false)
+        expect(object.instance_variable_defined?(:"@#{member_name}")).to be(false)
+      end
     end
+
+    it_behaves_like 'clearing memoization', :method_name
+    it_behaves_like 'clearing memoization', :enabled?
   end
 
   describe '.strong_memoize_attr' do
@@ -209,7 +219,6 @@ def public_method; end
 
       context "memoized after method definition with value #{value}" do
         let(:method_name) { :method_name_attr }
-        let(:member_name) { :method_name_attr }
 
         it_behaves_like 'caching the value'
 
@@ -218,30 +227,7 @@ def public_method; end
         end
 
         it 'retains method arity' do
-          expect(klass.instance_method(member_name).arity).to eq(0)
-        end
-      end
-
-      context "memoized before method definition with different member name and value #{value}" do
-        let(:method_name) { :different_method_name_attr }
-        let(:member_name) { :different_member_name_attr }
-
-        it_behaves_like 'caching the value'
-
-        it 'calls the existing .method_added' do
-          expect(klass.method_added_list).to include(:different_method_name_attr)
-        end
-      end
-
-      context 'with valid method name' do
-        let(:method_name) { :enabled? }
-
-        context 'with invalid member name' do
-          let(:member_name) { :enabled? }
-
-          it 'is invalid' do
-            expect { object.send(method_name) { value } }.to raise_error /is not allowed as an instance variable name/
-          end
+          expect(klass.instance_method(method_name).arity).to eq(0)
         end
       end
     end
@@ -299,4 +285,41 @@ def method_with_parameters(params); end
       end
     end
   end
+
+  describe '.normalize_key' do
+    using RSpec::Parameterized::TableSyntax
+
+    subject { described_class.normalize_key(input) }
+
+    where(:input, :output, :valid) do
+      :key    | :key    | true
+      "key"   | "key"   | true
+      :key?   | "key？" | true
+      "key?"  | "key？" | true
+      :key!   | "key！" | true
+      "key!"  | "key！" | true
+      # invalid cases caught elsewhere
+      :"ke?y" | :"ke?y" | false
+      "ke?y"  | "ke?y"  | false
+      :"ke!y" | :"ke!y" | false
+      "ke!y"  | "ke!y"  | false
+    end
+
+    with_them do
+      let(:ivar) { "@#{output}" }
+
+      it { is_expected.to eq(output) }
+
+      if params[:valid]
+        it 'is a valid ivar name' do
+          expect { instance_variable_defined?(ivar) }.not_to raise_error
+        end
+      else
+        it 'raises a NameError error' do
+          expect { instance_variable_defined?(ivar) }
+            .to raise_error(NameError, /not allowed as an instance/)
+        end
+      end
+    end
+  end
 end

spec/rubocop/cop/gitlab/strong_memoize_attr_spec.rb

@@ -47,7 +47,7 @@ class Foo
             def enabled?
               true
             end
-            strong_memoize_attr :enabled?, :enabled
+            strong_memoize_attr :enabled?
           end
         RUBY
       end