From e57c3fe287a91490f83a16d02a8455c058b055dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Matthias=20K=C3=A4ppler?= <mkaeppler@gitlab.com>
Date: Thu, 22 Jul 2021 09:35:05 +0000
Subject: [PATCH] Flag ActiveSupport::JSON in Gitlab::Json Cop
---
.rubocop.yml | 1 -
...90517153211_migrate_k8s_service_integration.rb | 2 +-
ee/spec/lib/gitlab/geo/oauth/session_spec.rb | 4 ++--
lib/gitlab/import_export/json/legacy_reader.rb | 2 +-
lib/gitlab/import_export/json/ndjson_reader.rb | 4 ++--
lib/gitlab/import_export/lfs_restorer.rb | 2 +-
lib/gitlab/json_cache.rb | 4 ++--
rubocop/cop/gitlab/json.rb | 2 +-
.../projects/import_export/export_file_spec.rb | 3 +--
.../group/legacy_tree_restorer_spec.rb | 2 +-
.../import_export/group/tree_restorer_spec.rb | 2 +-
.../import_export/import_test_coverage_spec.rb | 2 +-
spec/lib/gitlab/json_cache_spec.rb | 6 +++---
spec/requests/git_http_spec.rb | 4 ++--
spec/rubocop/cop/gitlab/json_spec.rb | 15 ++++++++++++++-
spec/support/import_export/common_util.rb | 6 +++---
16 files changed, 36 insertions(+), 25 deletions(-)
diff --git a/.rubocop.yml b/.rubocop.yml
index d34b133edee9f..19f0b0b294fbd 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -254,7 +254,6 @@ Gitlab/HTTParty:
Gitlab/Json:
Enabled: true
Exclude:
- - 'db/**/*'
- 'qa/**/*'
- 'scripts/**/*'
- 'tooling/rspec_flaky/**/*'
diff --git a/db/post_migrate/20190517153211_migrate_k8s_service_integration.rb b/db/post_migrate/20190517153211_migrate_k8s_service_integration.rb
index 4bd04edb23952..0b409cd286605 100644
--- a/db/post_migrate/20190517153211_migrate_k8s_service_integration.rb
+++ b/db/post_migrate/20190517153211_migrate_k8s_service_integration.rb
@@ -70,7 +70,7 @@ def token
private
def parsed_properties
- @parsed_properties ||= JSON.parse(self.properties)
+ @parsed_properties ||= JSON.parse(self.properties) # rubocop:disable Gitlab/Json
end
end
diff --git a/ee/spec/lib/gitlab/geo/oauth/session_spec.rb b/ee/spec/lib/gitlab/geo/oauth/session_spec.rb
index 5c8378c2653a0..4e8225a87ef5b 100644
--- a/ee/spec/lib/gitlab/geo/oauth/session_spec.rb
+++ b/ee/spec/lib/gitlab/geo/oauth/session_spec.rb
@@ -49,7 +49,7 @@ def stub_relative_url(host, script_name)
describe '#authenticate' do
let(:api_url) { "#{primary_node.internal_url.chomp('/')}/api/v4/user" }
- let(:user_json) { ActiveSupport::JSON.encode({ id: 555, email: 'user@example.com' }.as_json) }
+ let(:user_json) { Gitlab::Json.dump({ id: 555, email: 'user@example.com' }.as_json) }
context 'on success' do
before do
@@ -96,7 +96,7 @@ def stub_relative_url(host, script_name)
describe '#get_token' do
context 'primary is configured with relative URL' do
it "makes the request to a primary's relative URL" do
- response = ActiveSupport::JSON.encode({ access_token: 'fake-token' }.as_json)
+ response = Gitlab::Json.dump({ access_token: 'fake-token' }.as_json)
primary_node.update!(url: 'http://example.com/gitlab/')
api_url = "#{primary_node.internal_url}oauth/token"
diff --git a/lib/gitlab/import_export/json/legacy_reader.rb b/lib/gitlab/import_export/json/legacy_reader.rb
index 97b34088e3ed9..dc80c92f507f2 100644
--- a/lib/gitlab/import_export/json/legacy_reader.rb
+++ b/lib/gitlab/import_export/json/legacy_reader.rb
@@ -27,7 +27,7 @@ def tree_hash
end
def read_hash
- ActiveSupport::JSON.decode(IO.read(@path))
+ Gitlab::Json.parse(IO.read(@path))
rescue StandardError => e
Gitlab::ErrorTracking.log_exception(e)
raise Gitlab::ImportExport::Error, 'Incorrect JSON format'
diff --git a/lib/gitlab/import_export/json/ndjson_reader.rb b/lib/gitlab/import_export/json/ndjson_reader.rb
index 4899bd3b0ee8b..510da61d3ab8e 100644
--- a/lib/gitlab/import_export/json/ndjson_reader.rb
+++ b/lib/gitlab/import_export/json/ndjson_reader.rb
@@ -47,8 +47,8 @@ def consume_relation(importable_path, key, mark_as_consumed: true)
private
def json_decode(string)
- ActiveSupport::JSON.decode(string)
- rescue ActiveSupport::JSON.parse_error => e
+ Gitlab::Json.parse(string)
+ rescue JSON::ParserError => e
Gitlab::ErrorTracking.log_exception(e)
raise Gitlab::ImportExport::Error, 'Incorrect JSON format'
end
diff --git a/lib/gitlab/import_export/lfs_restorer.rb b/lib/gitlab/import_export/lfs_restorer.rb
index d73ae1410a355..9931b09e9cafa 100644
--- a/lib/gitlab/import_export/lfs_restorer.rb
+++ b/lib/gitlab/import_export/lfs_restorer.rb
@@ -72,7 +72,7 @@ def lfs_json
@lfs_json ||=
begin
json = IO.read(lfs_json_path)
- ActiveSupport::JSON.decode(json)
+ Gitlab::Json.parse(json)
rescue StandardError
raise Gitlab::ImportExport::Error, 'Incorrect JSON format'
end
diff --git a/lib/gitlab/json_cache.rb b/lib/gitlab/json_cache.rb
index 4314c131adadf..41c18f82a4b8c 100644
--- a/lib/gitlab/json_cache.rb
+++ b/lib/gitlab/json_cache.rb
@@ -58,7 +58,7 @@ def fetch(key, options = {}, &block)
private
def parse_value(raw, klass)
- value = ActiveSupport::JSON.decode(raw.to_s)
+ value = Gitlab::Json.parse(raw.to_s)
case value
when Hash then parse_entry(value, klass)
@@ -66,7 +66,7 @@ def parse_value(raw, klass)
else
value
end
- rescue ActiveSupport::JSON.parse_error
+ rescue JSON::ParserError
nil
end
diff --git a/rubocop/cop/gitlab/json.rb b/rubocop/cop/gitlab/json.rb
index 7cc719aca098b..d2ba0012ca0be 100644
--- a/rubocop/cop/gitlab/json.rb
+++ b/rubocop/cop/gitlab/json.rb
@@ -10,7 +10,7 @@ class Json < RuboCop::Cop::Cop
EOL
def_node_matcher :json_node?, <<~PATTERN
- (send (const nil? :JSON)...)
+ (send (const {nil? | (const nil? :ActiveSupport)} :JSON)...)
PATTERN
def on_send(node)
diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb
index 7f8ded4fa4321..ccf3ccc6a961d 100644
--- a/spec/features/projects/import_export/export_file_spec.rb
+++ b/spec/features/projects/import_export/export_file_spec.rb
@@ -82,8 +82,7 @@
relations << Gitlab::Json.parse(IO.read(project_json_path))
Dir.glob(File.join(tmpdir, 'tree/project', '*.ndjson')) do |rb_filename|
File.foreach(rb_filename) do |line|
- json = ActiveSupport::JSON.decode(line)
- relations << json
+ relations << Gitlab::Json.parse(line)
end
end
diff --git a/spec/lib/gitlab/import_export/group/legacy_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/group/legacy_tree_restorer_spec.rb
index bfcd4994995b4..dbd6cb243f6ed 100644
--- a/spec/lib/gitlab/import_export/group/legacy_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/group/legacy_tree_restorer_spec.rb
@@ -77,7 +77,7 @@
let(:group) { create(:group) }
let(:shared) { Gitlab::ImportExport::Shared.new(group) }
let(:group_tree_restorer) { described_class.new(user: importer_user, shared: shared, group: group, group_hash: nil) }
- let(:group_json) { ActiveSupport::JSON.decode(IO.read(File.join(shared.export_path, 'group.json'))) }
+ let(:group_json) { Gitlab::Json.parse(IO.read(File.join(shared.export_path, 'group.json'))) }
shared_examples 'excluded attributes' do
excluded_attributes = %w[
diff --git a/spec/lib/gitlab/import_export/group/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/group/tree_restorer_spec.rb
index d2153221e8fd2..b67d42d1b71fc 100644
--- a/spec/lib/gitlab/import_export/group/tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/group/tree_restorer_spec.rb
@@ -111,7 +111,7 @@
let(:shared) { Gitlab::ImportExport::Shared.new(group) }
let(:group_tree_restorer) { described_class.new(user: importer_user, shared: shared, group: group) }
let(:exported_file) { File.join(shared.export_path, 'tree/groups/4352.json') }
- let(:group_json) { ActiveSupport::JSON.decode(IO.read(exported_file)) }
+ let(:group_json) { Gitlab::Json.parse(IO.read(exported_file)) }
shared_examples 'excluded attributes' do
excluded_attributes = %w[
diff --git a/spec/lib/gitlab/import_export/import_test_coverage_spec.rb b/spec/lib/gitlab/import_export/import_test_coverage_spec.rb
index 9c6d270860777..90966cb491503 100644
--- a/spec/lib/gitlab/import_export/import_test_coverage_spec.rb
+++ b/spec/lib/gitlab/import_export/import_test_coverage_spec.rb
@@ -86,7 +86,7 @@ def tested_relations
end
def relations_from_json(json_file)
- json = ActiveSupport::JSON.decode(IO.read(json_file))
+ json = Gitlab::Json.parse(IO.read(json_file))
[].tap {|res| gather_relations({ project: json }, res, [])}
.map {|relation_names| relation_names.join('.')}
diff --git a/spec/lib/gitlab/json_cache_spec.rb b/spec/lib/gitlab/json_cache_spec.rb
index 8265c3449bb71..7899d01b4759d 100644
--- a/spec/lib/gitlab/json_cache_spec.rb
+++ b/spec/lib/gitlab/json_cache_spec.rb
@@ -130,7 +130,7 @@
.with(expanded_key)
.and_return(nil)
- expect(ActiveSupport::JSON).not_to receive(:decode)
+ expect(Gitlab::Json).not_to receive(:parse)
expect(cache.read(key)).to be_nil
end
@@ -140,7 +140,7 @@
.with(expanded_key)
.and_return(true)
- expect(ActiveSupport::JSON).to receive(:decode).with("true").and_call_original
+ expect(Gitlab::Json).to receive(:parse).with("true").and_call_original
expect(cache.read(key, BroadcastMessage)).to eq(true)
end
end
@@ -151,7 +151,7 @@
.with(expanded_key)
.and_return(false)
- expect(ActiveSupport::JSON).to receive(:decode).with("false").and_call_original
+ expect(Gitlab::Json).to receive(:parse).with("false").and_call_original
expect(cache.read(key, BroadcastMessage)).to eq(false)
end
end
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 3fb683ea0fa8e..2a91dae32b81b 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -267,7 +267,7 @@
it "responds to pulls with the wiki's repo" do
download(path) do |response|
- json_body = ActiveSupport::JSON.decode(response.body)
+ json_body = Gitlab::Json.parse(response.body)
expect(json_body['Repository']['relative_path']).to eq(wiki.repository.relative_path)
end
@@ -1584,7 +1584,7 @@ def attempt_login(include_password)
it "responds to pulls with the wiki's repo" do
download(path) do |response|
- json_body = ActiveSupport::JSON.decode(response.body)
+ json_body = Gitlab::Json.parse(response.body)
expect(json_body['Repository']['relative_path']).to eq(wiki.repository.relative_path)
end
diff --git a/spec/rubocop/cop/gitlab/json_spec.rb b/spec/rubocop/cop/gitlab/json_spec.rb
index 66b2c675e80ef..7998f26da4eac 100644
--- a/spec/rubocop/cop/gitlab/json_spec.rb
+++ b/spec/rubocop/cop/gitlab/json_spec.rb
@@ -6,7 +6,7 @@
RSpec.describe RuboCop::Cop::Gitlab::Json do
subject(:cop) { described_class.new }
- context 'when JSON is called' do
+ context 'when ::JSON is called' do
it 'registers an offense' do
expect_offense(<<~RUBY)
class Foo
@@ -18,4 +18,17 @@ def bar
RUBY
end
end
+
+ context 'when ActiveSupport::JSON is called' do
+ it 'registers an offense' do
+ expect_offense(<<~RUBY)
+ class Foo
+ def bar
+ ActiveSupport::JSON.parse('{ "foo": "bar" }')
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid calling `JSON` directly. [...]
+ end
+ end
+ RUBY
+ end
+ end
end
diff --git a/spec/support/import_export/common_util.rb b/spec/support/import_export/common_util.rb
index 5fb6af99b792b..1aa20dab6f8c7 100644
--- a/spec/support/import_export/common_util.rb
+++ b/spec/support/import_export/common_util.rb
@@ -83,7 +83,7 @@ def consume_attributes(dir_path, exportable_path)
path = File.join(dir_path, "#{exportable_path}.json")
return unless File.exist?(path)
- ActiveSupport::JSON.decode(IO.read(path))
+ Gitlab::Json.parse(IO.read(path))
end
def consume_relations(dir_path, exportable_path, key)
@@ -93,7 +93,7 @@ def consume_relations(dir_path, exportable_path, key)
relations = []
File.foreach(path) do |line|
- json = ActiveSupport::JSON.decode(line)
+ json = Gitlab::Json.parse(line)
relations << json
end
@@ -101,7 +101,7 @@ def consume_relations(dir_path, exportable_path, key)
end
def project_json(filename)
- ActiveSupport::JSON.decode(IO.read(filename))
+ Gitlab::Json.parse(IO.read(filename))
end
end
end
--
GitLab