Skip to content

Enable cops in the Lint Department #694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ gemspec
gem "diffy"
gem "minitest"
gem "rake"
gem "rubocop-minitest"
8 changes: 7 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
PATH
remote: .
specs:
rubocop-shopify (2.15.1)
rubocop-shopify (3.0.0)
lint_roller
rubocop (~> 1.72, >= 1.72.1)
rubocop-thread_safety (>= 0.7.1)

Expand Down Expand Up @@ -37,6 +38,10 @@ GEM
rubocop-ast (1.43.0)
parser (>= 3.3.7.2)
prism (~> 1.4)
rubocop-minitest (0.37.1)
lint_roller (~> 1.1)
rubocop (>= 1.72.1, < 2.0)
rubocop-ast (>= 1.38.0, < 2.0)
rubocop-thread_safety (0.7.2)
lint_roller (~> 1.1)
rubocop (~> 1.72, >= 1.72.1)
Expand All @@ -52,6 +57,7 @@ DEPENDENCIES
diffy
minitest
rake
rubocop-minitest
rubocop-shopify!

BUNDLED WITH
Expand Down
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Lint/NoReturnInMemoization:
Enabled: true
VersionAdded: "3.0.0"
Description: "Checks for the use of a `return` with a value in `begin..end` blocks in the context of instance variable assignment such as memoization."
6 changes: 6 additions & 0 deletions lib/rubocop-shopify.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# frozen_string_literal: true

require "rubocop/shopify/version"
require "rubocop/shopify/plugin"

require "rubocop/cop/lint/no_return_in_memoization"
98 changes: 98 additions & 0 deletions lib/rubocop/cop/lint/no_return_in_memoization.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# frozen_string_literal: true

require "rubocop"

module RuboCop
module Cop
module Lint
# Checks for the use of a `return` with a value in `begin..end` blocks
# in the context of instance variable assignment such as memoization.
# Using `return` with a value in these blocks can lead to unexpected behavior
# as the `return` will exit the method that contains and not set the values
# of the instance variable.
#
# @example
#
# # bad
# def foo
# @foo ||= begin
# return 1 if bar
# 2
# end
# end
#
# # bad
# def foo
# @foo = begin
# return 1 if bar
# 2
# end
# end
#
# # bad
# def foo
# @foo += begin
# return 1 if bar
# 2
# end
# end
#
# # bad - using return in rescue blocks still exits the method
# def foo
# @foo ||= begin
# bar
# rescue
# return 2 if baz
# end
# end
#
# # good
# def foo
# @foo ||= begin
# if bar
# 1
# else
# 2
# end
# end
# end
#
# # good - not an assignment to an instance variable
# def foo
# foo = begin
# return 1 if bar
# 2
# end
# end
#
# # good - proper exception handling without return
# def foo
# @foo ||= begin
# bar
# rescue
# baz ? 2 : 3
# end
# end
class NoReturnInMemoization < ::RuboCop::Cop::Base
MSG = 'Do not `return` in `begin..end` blocks in instance variable assignment contexts.'

def on_or_asgn(node)
node.each_node(:kwbegin) do |kwbegin_node|
kwbegin_node.each_node(:return) do |return_node|
add_offense(return_node)
end
end
end

def on_op_asgn(node)
return unless node.assignment_node.ivasgn_type?

on_or_asgn(node)
end

alias on_ivasgn on_or_asgn
alias on_and_asgn on_or_asgn
end
end
end
end
30 changes: 30 additions & 0 deletions lib/rubocop/shopify/plugin.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# frozen_string_literal: true

require "lint_roller"

module RuboCop
module Shopify
class Plugin < LintRoller::Plugin
def about
LintRoller::About.new(
name: "rubocop-shopify",
version: RuboCop::Shopify::VERSION,
homepage: "https://github.com/Shopify/rubocop-shopify",
description: "A plugin for RuboCop to enforce Shopify-specific coding standards."
)
end

def supported?(context)
context.engine == :rubocop
end

def rules(context)
LintRoller::Rules.new(
type: :path,
config_format: :rubocop,
value: File.expand_path("../../../config/default.yml", __dir__)
)
end
end
end
end
7 changes: 7 additions & 0 deletions lib/rubocop/shopify/version.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

module RuboCop
module Shopify
VERSION = "3.0.0"
end
end
11 changes: 9 additions & 2 deletions rubocop-shopify.gemspec
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
# frozen_string_literal: true

lib = File.expand_path("../lib", __FILE__)
$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)

require "rubocop/shopify/version"

Gem::Specification.new do |s|
s.platform = Gem::Platform::RUBY
s.name = "rubocop-shopify"
s.version = "2.15.1"
s.version = RuboCop::Shopify::VERSION
s.summary = "Shopify's style guide for Ruby."
s.description = "Gem containing the rubocop.yml config that corresponds to " \
"the implementation of the Shopify's style guide for Ruby."
Expand All @@ -14,15 +19,17 @@ Gem::Specification.new do |s|
s.email = "[email protected]"
s.homepage = "https://shopify.github.io/ruby-style-guide/"

s.files = Dir["rubocop*.yml", "lib/**/*", "LICENSE.md", "README.md"]
s.files = Dir["rubocop*.yml", "lib/**/*", "LICENSE.md", "config/default.yml", "README.md"]

s.metadata = {
"source_code_uri" => "https://github.com/Shopify/ruby-style-guide/tree/v#{s.version}",
"allowed_push_host" => "https://rubygems.org",
"default_lint_roller_plugin" => "RuboCop::Shopify::Plugin"
}

s.required_ruby_version = ">= 3.2.0"

s.add_dependency("rubocop", "~> 1.72", ">= 1.72.1")
s.add_dependency("rubocop-thread_safety", ">= 0.7.1")
s.add_dependency("lint_roller")
end
Loading