Skip to content
Snippets Groups Projects
Verified Commit 99e2f6fe authored by Peter Leitzen's avatar Peter Leitzen :three:
Browse files

Add new cop Gitlab/Rails/SafeFormat

Enforce `safe_format` for externalized strings with interpolations and
`.html_safe`.

  # bad
  _('string %{open}foo%{close}').html_safe % { open: '<b>'.html_safe, close: '</b>'.html_safe }
  format(_('string %{open}foo%{close}').html_safe, open: '<b>'.html_safe, close: '</b>'.html_safe)

  # good
  safe_format(_('string %{open}foo%{close}'), tag_pair(tag.b, :open, :close)
  # also good no `html_safe`
  format(_('string %{var} number'), var: var)
parent 3820b605
No related branches found
No related tags found
1 merge request!121313Add new cop Gitlab/Rails/SafeFormat
......@@ -116,6 +116,7 @@ linters:
- Cop/ProjectPathHelper
- Gitlab/FeatureAvailableUsage
- Gitlab/Json
- Gitlab/Rails/SafeFormat
- GitlabSecurity/PublicSend
- Layout/FirstHashElementIndentation
- Layout/EmptyLineAfterGuardClause
......
---
# Cop supports --autocorrect.
Gitlab/Rails/SafeFormat:
Details: grace period
Exclude:
- 'app/controllers/profiles/two_factor_auths_controller.rb'
- 'app/graphql/types/project_type.rb'
- 'app/helpers/auth_helper.rb'
- 'app/helpers/emails_helper.rb'
- 'app/helpers/groups/group_members_helper.rb'
- 'app/helpers/groups_helper.rb'
- 'app/helpers/import_helper.rb'
- 'app/helpers/members_helper.rb'
- 'app/helpers/merge_requests_helper.rb'
- 'app/helpers/profiles_helper.rb'
- 'app/helpers/projects_helper.rb'
- 'app/helpers/reminder_emails_helper.rb'
- 'app/helpers/search_helper.rb'
- 'app/helpers/sourcegraph_helper.rb'
- 'app/helpers/whats_new_helper.rb'
- 'app/models/integrations/apple_app_store.rb'
- 'app/models/integrations/asana.rb'
- 'app/models/integrations/bamboo.rb'
- 'app/models/integrations/beyond_identity.rb'
- 'app/models/integrations/bugzilla.rb'
- 'app/models/integrations/clickup.rb'
- 'app/models/integrations/confluence.rb'
- 'app/models/integrations/custom_issue_tracker.rb'
- 'app/models/integrations/datadog.rb'
- 'app/models/integrations/discord.rb'
- 'app/models/integrations/ewm.rb'
- 'app/models/integrations/external_wiki.rb'
- 'app/models/integrations/google_play.rb'
- 'app/models/integrations/hangouts_chat.rb'
- 'app/models/integrations/irker.rb'
- 'app/models/integrations/jenkins.rb'
- 'app/models/integrations/mattermost.rb'
- 'app/models/integrations/pivotaltracker.rb'
- 'app/models/integrations/redmine.rb'
- 'app/models/integrations/unify_circuit.rb'
- 'app/models/integrations/youtrack.rb'
- 'app/presenters/ci/pipeline_presenter.rb'
- 'app/presenters/key_presenter.rb'
- 'app/presenters/project_presenter.rb'
- 'app/services/jira/requests/base.rb'
- 'app/services/security/ci_configuration/base_create_service.rb'
- 'ee/app/components/namespaces/free_user_cap/enforcement_alert_component.rb'
- 'ee/app/components/namespaces/free_user_cap/usage_quota_alert_component.rb'
- 'ee/app/components/namespaces/free_user_cap/usage_quota_trial_alert_component.rb'
- 'ee/app/helpers/ee/application_helper.rb'
- 'ee/app/helpers/ee/import_helper.rb'
- 'ee/app/helpers/ee/members_helper.rb'
- 'ee/app/helpers/ee/search_helper.rb'
- 'ee/app/helpers/push_rules_helper.rb'
- 'ee/app/models/integrations/git_guardian.rb'
- 'ee/app/models/integrations/github.rb'
- 'ee/lib/gitlab/licenses/submit_license_usage_data_banner.rb'
- 'ee/lib/gitlab/manual_quarterly_co_term_banner.rb'
- 'spec/helpers/profiles_helper_spec.rb'
# frozen_string_literal: true
module RuboCop
module Cop
module Gitlab
module Rails
# Enforce `safe_format` for externalized strings with interpolations and `.html_safe`.
#
# @example
# # bad
# _('string %{open}foo%{close}').html_safe % { open: '<b>'.html_safe, close: '</b>'.html_safe }
# format(_('string %{open}foo%{close}').html_safe, open: '<b>'.html_safe, close: '</b>'.html_safe)
# _('foo').html_safe
#
# # good
# safe_format(_('string %{open}foo%{close}'), tag_pair(tag.b, :open, :close))
# safe_format('foo')
#
# # also good no `html_safe`
# format(_('string %{var} number'), var: var)
# _('string %{var} number') % { var: var }
class SafeFormat < RuboCop::Cop::Base
extend RuboCop::Cop::AutoCorrector
MSG = 'Use `safe_format` to interpolate externalized strings. ' \
'See https://docs.gitlab.com/ee/development/i18n/externalization.html#html'
RESTRICT_ON_SEND = %i[_ s_ N_ n_].freeze
def_node_matcher :wrapped_by?, <<~PATTERN
^(send _ %method ...)
PATTERN
def on_send(gettext)
return unless wrapped_by?(gettext, method: :html_safe)
return if wrapped_by?(gettext, method: :safe_format)
call, args = find_call_and_args(gettext)
node_to_replace = call
node_to_replace = node_to_replace.parent if wrapped_by?(node_to_replace, method: :html_safe)
node_to_replace = node_to_replace.parent if wrapped_by?(node_to_replace, method: :html_escape)
add_offense(call.loc.selector) do |corrector|
use_safe_format(corrector, node_to_replace, gettext, args)
end
end
private
def find_call_and_args(node)
call = node
args = []
node.each_ancestor do |ancestor|
break unless ancestor.send_type?
case ancestor.send_type? && ancestor.method_name
when :format
call = ancestor
args = ancestor.arguments.drop(1)
break
when :%
call = ancestor
args = ancestor.arguments
break
end
end
[call, args]
end
def use_safe_format(corrector, node, gettext, args)
receiver = gettext.source
if args&.any?
args = unwrap_args(args)
.then { |args| use_tag_pair(args) }
.then { |args| sourcify_args(args) }
corrector.replace(node, "safe_format(#{receiver}, #{args})")
else
corrector.replace(node, "safe_format(#{receiver})")
end
end
def unwrap_args(args)
return args[0].children if args[0].array_type? || args[0].hash_type?
args
end
# Turns `open: '<b>'.html_safe, close: '</b>'.html_safe` into
# `tag_pair(tag.b, :open, :close)`.
def use_tag_pair(args)
return args unless args.all?(&:pair_type?)
pair_hash = args.to_h { |pair| [pair.key, pair.value] }
seen = Hash.new { |hash, tag| hash[tag] = [] }
tag_pairs = []
args.each do |pair|
# We only care about { a: '<b>'.html_safe }. Ignore the rest
next unless pair.value.send_type? && pair.value.method?(:html_safe) && pair.value.receiver.str_type?
# Extract the tag from `<b>` or `</b>`.
tag = pair.value.receiver.value[%r{^</?(\w+)>}, 1]
next unless tag
seen[tag] << pair
next unless seen[tag].size == 2
closing_tag, opening_tag = seen[tag].sort_by { |pair| pair.value.receiver.value }
pair_hash.delete(closing_tag.key)
pair_hash.delete(opening_tag.key)
keys = [opening_tag, closing_tag].map { |pair| pair.key.value.inspect }.join(", ")
tag_pairs << "tag_pair(tag.#{tag}, #{keys})"
seen[tag].clear
end
tag_pairs + pair_hash.keys.map(&:parent)
end
def sourcify_args(args)
args.map { |arg| arg.respond_to?(:source) ? arg.source : arg }.join(', ')
end
end
end
end
end
end
# frozen_string_literal: true
require 'rubocop_spec_helper'
require_relative '../../../../../rubocop/cop/gitlab/rails/safe_format'
RSpec.describe RuboCop::Cop::Gitlab::Rails::SafeFormat, feature_category: :tooling do
shared_examples 'safe formatted externalized string' do |method:, condition: ""|
context "for gettext method `#{method}(...)#{condition}`" do
context 'with String#% and hash arg' do
it 'flags and autocorrects externalized strings' do
expect_offense(<<~RUBY, method: method, condition: condition)
%{method}('%{a}string %{open}foo%{close}').html_safe % { a: '1'.html_safe, open: '<b>'.html_safe, close: '</b>'.html_safe }%{condition}
_{method} ^ Use `safe_format` [...]
html_escape(%{method}('%{a}string %{open}foo%{close}').html_safe) %
_{method} ^ Use `safe_format` [...]
{ a: '1'.html_safe, open: '<b>'.html_safe, close: '</b>'.html_safe }%{condition}
RUBY
expect_correction(<<~RUBY)
safe_format(#{method}('%{a}string %{open}foo%{close}'), tag_pair(tag.b, :open, :close), a: '1'.html_safe)#{condition}
safe_format(#{method}('%{a}string %{open}foo%{close}'), tag_pair(tag.b, :open, :close), a: '1'.html_safe)#{condition}
RUBY
end
end
context 'with String#% and array arg' do
it 'flags and autocorrects externalized strings' do
expect_offense(<<~RUBY, method: method, condition: condition)
%{method}('string %sfoo%s').html_safe % [ '<b>'.html_safe, '</b>'.html_safe ]%{condition}
_{method} ^ Use `safe_format` [...]
html_escape(%{method}('string %sfoo%s').html_safe) % ['<b>'.html_safe, '</b>'.html_safe]%{condition}
_{method} ^ Use `safe_format` [...]
RUBY
expect_correction(<<~RUBY)
safe_format(#{method}('string %sfoo%s'), '<b>'.html_safe, '</b>'.html_safe)#{condition}
safe_format(#{method}('string %sfoo%s'), '<b>'.html_safe, '</b>'.html_safe)#{condition}
RUBY
end
end
context 'with String#% and bare arg' do
it 'flags and autocorrects externalized strings' do
expect_offense(<<~RUBY, method: method, condition: condition)
%{method}('string %sfoo').html_safe % '<b>'.html_safe%{condition}
_{method} ^ Use `safe_format` [...]
html_escape(%{method}('string %sfoo').html_safe) % '<b>'.html_safe%{condition}
_{method} ^ Use `safe_format` [...]
RUBY
expect_correction(<<~RUBY)
safe_format(#{method}('string %sfoo'), '<b>'.html_safe)#{condition}
safe_format(#{method}('string %sfoo'), '<b>'.html_safe)#{condition}
RUBY
end
end
context 'with String#% and no html_safe' do
it 'does not flag' do
expect_no_offenses(<<~RUBY)
#{method}('string %{name}') % { name: name }#{condition}
RUBY
end
end
context 'with String#format and hash arg' do
it 'flags and autocorrects externalized strings' do
expect_offense(<<~RUBY, method: method, condition: condition)
format(%{method}('%{a}string %{open}foo%{close}').html_safe, a: '1'.html_safe, open: '<b>'.html_safe, close: '</b>'.html_safe)%{condition}
^^^^^^ Use `safe_format` [...]
html_escape(format(%{method}('%{a}string %{open}foo%{close}').html_safe, a: '1'.html_safe, open: '<b>'.html_safe, close: '</b>'.html_safe))%{condition}
^^^^^^ Use `safe_format` [...]
RUBY
expect_correction(<<~RUBY)
safe_format(#{method}('%{a}string %{open}foo%{close}'), tag_pair(tag.b, :open, :close), a: '1'.html_safe)#{condition}
safe_format(#{method}('%{a}string %{open}foo%{close}'), tag_pair(tag.b, :open, :close), a: '1'.html_safe)#{condition}
RUBY
end
end
context 'with String#format and no arg' do
it 'flags and autocorrects externalized strings' do
expect_offense(<<~RUBY, method: method, condition: condition)
format(%{method}('string').html_safe)%{condition}
^^^^^^ Use `safe_format` [...]
html_escape(format(%{method}('string').html_safe))%{condition}
^^^^^^ Use `safe_format` [...]
RUBY
expect_correction(<<~RUBY)
safe_format(#{method}('string'))#{condition}
safe_format(#{method}('string'))#{condition}
RUBY
end
end
context 'with String#format and no html_safe' do
it 'does not flag' do
expect_no_offenses(<<~RUBY)
format(#{method}('string %{name}'), name: name)#{condition}
RUBY
end
end
context 'with bare calls' do
it 'flags and autocorrects externalized strings' do
expect_offense(<<~RUBY, method: method, condition: condition)
%{method}('string').html_safe%{condition}
^{method} Use `safe_format` [...]
html_escape(%{method}('string').html_safe)%{condition}
^{method} Use `safe_format` [...]
RUBY
expect_correction(<<~RUBY)
safe_format(#{method}('string'))#{condition}
safe_format(#{method}('string'))#{condition}
RUBY
end
it 'does not flag' do
expect_no_offenses(<<~RUBY)
#{method}('string')#{condition}
RUBY
end
end
end
end
it_behaves_like 'safe formatted externalized string', method: :_
it_behaves_like 'safe formatted externalized string', method: :_, condition: ' if cond'
it_behaves_like 'safe formatted externalized string', method: :s_
it_behaves_like 'safe formatted externalized string', method: :s_, condition: ' unless cond'
it_behaves_like 'safe formatted externalized string', method: :n_
it_behaves_like 'safe formatted externalized string', method: :n_, condition: ' if cond'
it_behaves_like 'safe formatted externalized string', method: :N_
it_behaves_like 'safe formatted externalized string', method: :N_, condition: ' unless cond'
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment