Fix Dangerfile parsing non-Ruby files

- For internal_users.rb

Changelog: fixed

danger/internal_users/Dangerfile

+# frozen_string_literal: true
+
+internal_users.add_comment_for_internal_users_changes

danger/plugins/internal_users.rb

+# frozen_string_literal: true
+
+require_relative '../../tooling/danger/internal_users'
+
+module Danger
+  class InternalUsers < Plugin
+    include Tooling::Danger::InternalUsers
+  end
+end

spec/tooling/danger/internal_users_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'gitlab/dangerfiles/spec_helper'
+
+require_relative '../../../tooling/danger/internal_users'
+require_relative '../../../tooling/danger/project_helper'
+
+RSpec.describe Tooling::Danger::InternalUsers, feature_category: :tooling do
+  include_context "with dangerfile"
+
+  let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
+  let(:fake_project_helper) { instance_double(Tooling::Danger::ProjectHelper) }
+  let(:file_path) { 'app/models/user.rb' }
+  let(:changed_lines) { [] }
+  let(:file_lines) { [] }
+
+  subject(:helper) { fake_danger.new(helper: fake_helper) }
+
+  before do
+    allow(helper).to receive(:project_helper).and_return(fake_project_helper)
+    allow(fake_helper).to receive(:all_changed_files).and_return([file_path])
+    allow(fake_helper).to receive(:changed_lines).with(file_path).and_return(changed_lines)
+    allow(fake_project_helper).to receive(:file_lines).with(file_path).and_return(file_lines)
+  end
+
+  describe '#add_comment_for_internal_users_changes' do
+    shared_examples 'no violations' do
+      it 'adds no comments' do
+        expect(helper).not_to receive(:markdown)
+        expect(helper).not_to receive(:warn)
+
+        helper.add_comment_for_internal_users_changes
+      end
+    end
+
+    shared_examples 'has violations' do
+      it 'adds file and MR level comments' do
+        expect(helper).to receive(:markdown).once
+        expect(helper).to receive(:warn).once
+
+        helper.add_comment_for_internal_users_changes
+      end
+    end
+
+    context 'when documentation is changed' do
+      before do
+        allow(fake_helper).to receive(:all_changed_files).and_return([described_class::DOCS_PATH])
+      end
+
+      include_examples 'no violations'
+    end
+
+    context 'when method name is changed' do
+      let(:changed_lines) do
+        [
+          '-  def security_bot',
+          '+  def security_bot_v2'
+        ]
+      end
+
+      let(:file_lines) { ['  def security_bot_v2'] }
+
+      include_examples 'has violations'
+    end
+
+    context 'when method body is changed' do
+      let(:changed_lines) do
+        [
+          '  def security_bot',
+          '+    super',
+          '  end'
+        ]
+      end
+
+      let(:file_lines) do
+        [
+          'def security_bot',
+          '  super',
+          'end'
+        ]
+      end
+
+      include_examples 'has violations'
+    end
+
+    context 'when bot module is referenced' do
+      let(:changed_lines) do
+        [
+          '+    Users::Internal.security_bot'
+        ]
+      end
+
+      let(:file_lines) { ['    Users::Internal.security_bot'] }
+
+      include_examples 'has violations'
+    end
+
+    context 'when bot symbol is used' do
+      let(:changed_lines) do
+        [
+          '+    user_type: :security_bot'
+        ]
+      end
+
+      let(:file_lines) { ['    user_type: :security_bot'] }
+
+      include_examples 'has violations'
+    end
+
+    context 'when changes are unrelated' do
+      let(:changed_lines) do
+        [
+          '+    user.name = "Regular User"'
+        ]
+      end
+
+      let(:file_lines) { ['    user.name = "Regular User"'] }
+
+      include_examples 'no violations'
+    end
+
+    context 'when changing lines inside a bot method definition' do
+      let(:changed_lines) do
+        [
+          '    user.name = "Bot Name"'
+        ]
+      end
+
+      let(:file_lines) do
+        [
+          'def security_bot',
+          '    user.name = "Bot Name"',
+          'end'
+        ]
+      end
+
+      include_examples 'has violations'
+    end
+
+    context 'when changing lines outside any bot method definition' do
+      let(:changed_lines) do
+        [
+          '    user.name = "Regular Name"'
+        ]
+      end
+
+      let(:file_lines) do
+        [
+          'def regular_method',
+          '    user.name = "Regular Name"',
+          'end'
+        ]
+      end
+
+      include_examples 'no violations'
+    end
+
+    context 'when bot method spans multiple ends' do
+      let(:changed_lines) do
+        [
+          '    user.tap do |u|',
+          '      u.name = "Bot Name"',
+          '    end'
+        ]
+      end
+
+      let(:file_lines) do
+        [
+          'def security_bot',
+          '    user.tap do |u|',
+          '      u.name = "Bot Name"',
+          '    end',
+          'end'
+        ]
+      end
+
+      include_examples 'has violations'
+    end
+
+    context 'when modifying code between bot methods' do
+      let(:changed_lines) do
+        [
+          '    user.name = "Regular Name"'
+        ]
+      end
+
+      let(:file_lines) do
+        [
+          'def security_bot',
+          '  # bot code',
+          'end',
+          '    user.name = "Regular Name"',
+          'def alert_bot',
+          '  # bot code',
+          'end'
+        ]
+      end
+
+      include_examples 'no violations'
+    end
+
+    context 'when file is not a ruby file' do
+      let(:file_path) { 'some_file.png' }
+      let(:changed_lines) { [] }
+
+      include_examples 'no violations'
+    end
+  end
+
+  describe '#file_has_violations?' do
+    context 'when tracking bot method boundaries' do
+      let(:changed_lines) { ['  some_code'] }
+
+      it 'correctly tracks entering and exiting bot methods' do
+        allow(fake_project_helper).to receive(:file_lines).and_return([
+          'def security_bot',
+          '  some_code',
+          'end'
+        ])
+
+        expect(helper.send(:file_has_violations?, file_path)).to be true
+
+        allow(fake_project_helper).to receive(:file_lines).and_return([
+          'def security_bot',
+          '  other_code',
+          'end',
+          '  some_code'
+        ])
+
+        expect(helper.send(:file_has_violations?, file_path)).to be false
+      end
+
+      it 'handles nested end keywords' do
+        allow(fake_project_helper).to receive(:file_lines).and_return([
+          'def security_bot',
+          '  some_code',
+          '  if true',
+          '    some_code',
+          '  end',
+          'end'
+        ])
+
+        expect(helper.send(:file_has_violations?, file_path)).to be true
+      end
+
+      it 'processes all lines in method body' do
+        lines = [
+          'def security_bot',
+          '  line1',
+          '  line2',
+          '  line3',
+          'end'
+        ]
+
+        allow(fake_project_helper).to receive(:file_lines).and_return(lines)
+        allow(fake_helper).to receive(:changed_lines).and_return(['  line2'])
+
+        expect(helper.send(:file_has_violations?, file_path)).to be true
+      end
+    end
+  end
+end

tooling/danger/internal_users.rb

+# frozen_string_literal: true
+
+require_relative 'suggestor'
+
+module Tooling
+  module Danger
+    module InternalUsers
+      include ::Tooling::Danger::Suggestor
+
+      DOCS_PATH = 'doc/administration/internal_users.md'
+
+      INLINE_COMMENT =
+        <<~MESSAGE.freeze
+        This line modifies internal user behavior or references an internal bot user.
+        If this introduces new capabilities or changes existing behavior, please update the [internal users documentation](#{DOCS_PATH}).
+
+        This comment will only appear once per file.
+        MESSAGE
+
+      MR_LEVEL_COMMENT =
+        <<~SUGGEST_COMMENT.freeze
+        This MR changes internal user behavior or usage.
+        Please ensure [internal users documentation](#{DOCS_PATH}) is up to date.
+
+        If this is not applicable, please ignore this message.
+
+        Consider documenting:
+        - Any changes to internal user behavior or capabilities
+        - New use cases or integrations
+        - Changes in how bots may interact with GitLab and its features
+        SUGGEST_COMMENT
+
+      BOT_METHODS = %w[
+        support_bot
+        alert_bot
+        visual_review_bot
+        ghost
+        migration_bot
+        security_bot
+        automation_bot
+        security_policy_bot
+        admin_bot
+        suggested_reviewers_bot
+        llm_bot
+        placeholder
+        duo_code_review_bot
+        import_user
+      ].freeze
+
+      BOT_NAMES = BOT_METHODS.join('|')
+
+      BOT_METHOD_DEF_REGEX = /^\s*def\s+(?:self\.)?\s*(#{BOT_NAMES})(?!\w)/
+      BOT_MODULE_CALL_REGEX = /^\s*Users::Internal\.(?:#{BOT_NAMES})(?!\w)/
+      BOT_SYMBOL_TYPE_REGEX = /^\s*(?!#|")[^#"]*:(?:#{BOT_NAMES})(?!\w)/
+
+      def add_comment_for_internal_users_changes
+        return if helper.all_changed_files.include?(DOCS_PATH)
+
+        violations_found = false
+
+        helper.all_changed_files.each do |filename|
+          next unless filename.end_with?('.rb')
+          next unless file_has_violations?(filename)
+
+          violations_found = true
+          markdown(INLINE_COMMENT, file: filename)
+        end
+
+        warn(MR_LEVEL_COMMENT) if violations_found
+      end
+
+      private
+
+      def file_has_violations?(filename)
+        changed_lines = helper.changed_lines(filename)
+        file_lines = project_helper.file_lines(filename)
+
+        in_bot_method = false
+        has_violation = false
+
+        changed_lines.each do |line|
+          clean_line = line.delete_prefix('+').delete_prefix('-').strip
+          next unless clean_line.match?(BOT_METHOD_DEF_REGEX) ||
+            clean_line.match?(BOT_MODULE_CALL_REGEX) ||
+            clean_line.match?(BOT_SYMBOL_TYPE_REGEX)
+
+          has_violation = true
+          break
+        end
+
+        return has_violation if has_violation
+
+        file_lines.each_with_index do |line, _|
+          if line.match?(BOT_METHOD_DEF_REGEX)
+            in_bot_method = true
+          elsif line.strip == 'end'
+            in_bot_method = false
+          end
+
+          next unless in_bot_method
+          next unless changed_lines.any? { |cl| cl.delete_prefix('+').delete_prefix('-').strip == line.strip }
+
+          has_violation = true
+          break
+        end
+
+        has_violation
+      end
+    end
+  end
+end