Commit 284fad3a authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch 'mk-json-cop-activesupport' into 'master'

Flag ActiveSupport::JSON in Gitlab::Json Cop

See merge request gitlab-org/gitlab!66531
parents e387173f e57c3fe2
...@@ -254,7 +254,6 @@ Gitlab/HTTParty: ...@@ -254,7 +254,6 @@ Gitlab/HTTParty:
Gitlab/Json: Gitlab/Json:
Enabled: true Enabled: true
Exclude: Exclude:
- 'db/**/*'
- 'qa/**/*' - 'qa/**/*'
- 'scripts/**/*' - 'scripts/**/*'
- 'tooling/rspec_flaky/**/*' - 'tooling/rspec_flaky/**/*'
......
...@@ -70,7 +70,7 @@ class MigrateK8sServiceIntegration < ActiveRecord::Migration[5.1] ...@@ -70,7 +70,7 @@ class MigrateK8sServiceIntegration < ActiveRecord::Migration[5.1]
private private
def parsed_properties def parsed_properties
@parsed_properties ||= JSON.parse(self.properties) @parsed_properties ||= JSON.parse(self.properties) # rubocop:disable Gitlab/Json
end end
end end
......
...@@ -49,7 +49,7 @@ RSpec.describe Gitlab::Geo::Oauth::Session, :geo do ...@@ -49,7 +49,7 @@ RSpec.describe Gitlab::Geo::Oauth::Session, :geo do
describe '#authenticate' do describe '#authenticate' do
let(:api_url) { "#{primary_node.internal_url.chomp('/')}/api/v4/user" } 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 context 'on success' do
before do before do
...@@ -96,7 +96,7 @@ RSpec.describe Gitlab::Geo::Oauth::Session, :geo do ...@@ -96,7 +96,7 @@ RSpec.describe Gitlab::Geo::Oauth::Session, :geo do
describe '#get_token' do describe '#get_token' do
context 'primary is configured with relative URL' do context 'primary is configured with relative URL' do
it "makes the request to a primary's 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/') primary_node.update!(url: 'http://example.com/gitlab/')
api_url = "#{primary_node.internal_url}oauth/token" api_url = "#{primary_node.internal_url}oauth/token"
......
...@@ -27,7 +27,7 @@ module Gitlab ...@@ -27,7 +27,7 @@ module Gitlab
end end
def read_hash def read_hash
ActiveSupport::JSON.decode(IO.read(@path)) Gitlab::Json.parse(IO.read(@path))
rescue StandardError => e rescue StandardError => e
Gitlab::ErrorTracking.log_exception(e) Gitlab::ErrorTracking.log_exception(e)
raise Gitlab::ImportExport::Error, 'Incorrect JSON format' raise Gitlab::ImportExport::Error, 'Incorrect JSON format'
......
...@@ -47,8 +47,8 @@ module Gitlab ...@@ -47,8 +47,8 @@ module Gitlab
private private
def json_decode(string) def json_decode(string)
ActiveSupport::JSON.decode(string) Gitlab::Json.parse(string)
rescue ActiveSupport::JSON.parse_error => e rescue JSON::ParserError => e
Gitlab::ErrorTracking.log_exception(e) Gitlab::ErrorTracking.log_exception(e)
raise Gitlab::ImportExport::Error, 'Incorrect JSON format' raise Gitlab::ImportExport::Error, 'Incorrect JSON format'
end end
......
...@@ -72,7 +72,7 @@ module Gitlab ...@@ -72,7 +72,7 @@ module Gitlab
@lfs_json ||= @lfs_json ||=
begin begin
json = IO.read(lfs_json_path) json = IO.read(lfs_json_path)
ActiveSupport::JSON.decode(json) Gitlab::Json.parse(json)
rescue StandardError rescue StandardError
raise Gitlab::ImportExport::Error, 'Incorrect JSON format' raise Gitlab::ImportExport::Error, 'Incorrect JSON format'
end end
......
...@@ -58,7 +58,7 @@ module Gitlab ...@@ -58,7 +58,7 @@ module Gitlab
private private
def parse_value(raw, klass) def parse_value(raw, klass)
value = ActiveSupport::JSON.decode(raw.to_s) value = Gitlab::Json.parse(raw.to_s)
case value case value
when Hash then parse_entry(value, klass) when Hash then parse_entry(value, klass)
...@@ -66,7 +66,7 @@ module Gitlab ...@@ -66,7 +66,7 @@ module Gitlab
else else
value value
end end
rescue ActiveSupport::JSON.parse_error rescue JSON::ParserError
nil nil
end end
......
...@@ -10,7 +10,7 @@ module RuboCop ...@@ -10,7 +10,7 @@ module RuboCop
EOL EOL
def_node_matcher :json_node?, <<~PATTERN def_node_matcher :json_node?, <<~PATTERN
(send (const nil? :JSON)...) (send (const {nil? | (const nil? :ActiveSupport)} :JSON)...)
PATTERN PATTERN
def on_send(node) def on_send(node)
......
...@@ -82,8 +82,7 @@ RSpec.describe 'Import/Export - project export integration test', :js do ...@@ -82,8 +82,7 @@ RSpec.describe 'Import/Export - project export integration test', :js do
relations << Gitlab::Json.parse(IO.read(project_json_path)) relations << Gitlab::Json.parse(IO.read(project_json_path))
Dir.glob(File.join(tmpdir, 'tree/project', '*.ndjson')) do |rb_filename| Dir.glob(File.join(tmpdir, 'tree/project', '*.ndjson')) do |rb_filename|
File.foreach(rb_filename) do |line| File.foreach(rb_filename) do |line|
json = ActiveSupport::JSON.decode(line) relations << Gitlab::Json.parse(line)
relations << json
end end
end end
......
...@@ -77,7 +77,7 @@ RSpec.describe Gitlab::ImportExport::Group::LegacyTreeRestorer do ...@@ -77,7 +77,7 @@ RSpec.describe Gitlab::ImportExport::Group::LegacyTreeRestorer do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:shared) { Gitlab::ImportExport::Shared.new(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_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 shared_examples 'excluded attributes' do
excluded_attributes = %w[ excluded_attributes = %w[
......
...@@ -111,7 +111,7 @@ RSpec.describe Gitlab::ImportExport::Group::TreeRestorer do ...@@ -111,7 +111,7 @@ RSpec.describe Gitlab::ImportExport::Group::TreeRestorer do
let(:shared) { Gitlab::ImportExport::Shared.new(group) } let(:shared) { Gitlab::ImportExport::Shared.new(group) }
let(:group_tree_restorer) { described_class.new(user: importer_user, shared: shared, group: 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(: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 shared_examples 'excluded attributes' do
excluded_attributes = %w[ excluded_attributes = %w[
......
...@@ -86,7 +86,7 @@ RSpec.describe 'Test coverage of the Project Import' do ...@@ -86,7 +86,7 @@ RSpec.describe 'Test coverage of the Project Import' do
end end
def relations_from_json(json_file) 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, [])} [].tap {|res| gather_relations({ project: json }, res, [])}
.map {|relation_names| relation_names.join('.')} .map {|relation_names| relation_names.join('.')}
......
...@@ -130,7 +130,7 @@ RSpec.describe Gitlab::JsonCache do ...@@ -130,7 +130,7 @@ RSpec.describe Gitlab::JsonCache do
.with(expanded_key) .with(expanded_key)
.and_return(nil) .and_return(nil)
expect(ActiveSupport::JSON).not_to receive(:decode) expect(Gitlab::Json).not_to receive(:parse)
expect(cache.read(key)).to be_nil expect(cache.read(key)).to be_nil
end end
...@@ -140,7 +140,7 @@ RSpec.describe Gitlab::JsonCache do ...@@ -140,7 +140,7 @@ RSpec.describe Gitlab::JsonCache do
.with(expanded_key) .with(expanded_key)
.and_return(true) .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) expect(cache.read(key, BroadcastMessage)).to eq(true)
end end
end end
...@@ -151,7 +151,7 @@ RSpec.describe Gitlab::JsonCache do ...@@ -151,7 +151,7 @@ RSpec.describe Gitlab::JsonCache do
.with(expanded_key) .with(expanded_key)
.and_return(false) .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) expect(cache.read(key, BroadcastMessage)).to eq(false)
end end
end end
......
...@@ -267,7 +267,7 @@ RSpec.describe 'Git HTTP requests' do ...@@ -267,7 +267,7 @@ RSpec.describe 'Git HTTP requests' do
it "responds to pulls with the wiki's repo" do it "responds to pulls with the wiki's repo" do
download(path) do |response| 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) expect(json_body['Repository']['relative_path']).to eq(wiki.repository.relative_path)
end end
...@@ -1584,7 +1584,7 @@ RSpec.describe 'Git HTTP requests' do ...@@ -1584,7 +1584,7 @@ RSpec.describe 'Git HTTP requests' do
it "responds to pulls with the wiki's repo" do it "responds to pulls with the wiki's repo" do
download(path) do |response| 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) expect(json_body['Repository']['relative_path']).to eq(wiki.repository.relative_path)
end end
......
...@@ -6,7 +6,7 @@ require_relative '../../../../rubocop/cop/gitlab/json' ...@@ -6,7 +6,7 @@ require_relative '../../../../rubocop/cop/gitlab/json'
RSpec.describe RuboCop::Cop::Gitlab::Json do RSpec.describe RuboCop::Cop::Gitlab::Json do
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context 'when JSON is called' do context 'when ::JSON is called' do
it 'registers an offense' do it 'registers an offense' do
expect_offense(<<~RUBY) expect_offense(<<~RUBY)
class Foo class Foo
...@@ -18,4 +18,17 @@ RSpec.describe RuboCop::Cop::Gitlab::Json do ...@@ -18,4 +18,17 @@ RSpec.describe RuboCop::Cop::Gitlab::Json do
RUBY RUBY
end end
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 end
...@@ -83,7 +83,7 @@ module ImportExport ...@@ -83,7 +83,7 @@ module ImportExport
path = File.join(dir_path, "#{exportable_path}.json") path = File.join(dir_path, "#{exportable_path}.json")
return unless File.exist?(path) return unless File.exist?(path)
ActiveSupport::JSON.decode(IO.read(path)) Gitlab::Json.parse(IO.read(path))
end end
def consume_relations(dir_path, exportable_path, key) def consume_relations(dir_path, exportable_path, key)
...@@ -93,7 +93,7 @@ module ImportExport ...@@ -93,7 +93,7 @@ module ImportExport
relations = [] relations = []
File.foreach(path) do |line| File.foreach(path) do |line|
json = ActiveSupport::JSON.decode(line) json = Gitlab::Json.parse(line)
relations << json relations << json
end end
...@@ -101,7 +101,7 @@ module ImportExport ...@@ -101,7 +101,7 @@ module ImportExport
end end
def project_json(filename) def project_json(filename)
ActiveSupport::JSON.decode(IO.read(filename)) Gitlab::Json.parse(IO.read(filename))
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment