Merge pull request #545 from Shopify/catlee/no_return_in_blocks

Enable the Lint/NoReturnInBeginEndBlocks cop

README.md

@@ -625,6 +625,31 @@ documentation](https://docs.rubocop.org/rubocop/configuration.html#inheriting-co
 * Prefer `Time.iso8601(foo)` instead of `Time.parse(foo)` when expecting ISO8601
   formatted time strings like `"2018-03-20T11:16:39-04:00"`.
 
+* Avoid returning from a `begin` block in assignment contexts. If you return
+  from a method inside a `begin` block, the return will prevent the assignment
+  from taking place, potentially causing confusing memoization bugs.
+
+  ~~~ ruby
+  # bad
+  def foo
+    @foo ||= begin
+      return 1 if flag?
+      2
+    end
+  end
+
+  # good
+  def foo
+    @foo ||= begin
+      if flag?
+        1
+      else
+        2
+      end
+    end
+  end
+  ~~~
+
 ## Naming
 
 * Use `snake_case` for symbols, methods, and variables.

rubocop.yml

@@ -195,7 +195,7 @@ Lint/NestedPercentLiteral:
   Enabled: false
 
 Lint/NoReturnInBeginEndBlocks:
-  Enabled: false
+  Enabled: true
 
 Lint/NonAtomicFileOperation:
   Enabled: false

test/fixtures/full_config.yml

@@ -1464,7 +1464,7 @@ Lint/NextWithoutAccumulator:
   VersionAdded: '0.36'
 Lint/NoReturnInBeginEndBlocks:
   Description: Do not `return` inside `begin..end` blocks in assignment contexts.
-  Enabled: false
+  Enabled: true
   VersionAdded: '1.2'
 Lint/NonAtomicFileOperation:
   Description: Checks for non-atomic file operations.