Skip to content

Commit

Permalink
Enabled frozen string literals (Shopify#1154)
Browse files Browse the repository at this point in the history
* Enabled frozen string literals

* Update rubocop config

* Prefer string interpolation in simple cases

Co-Authored-By: Dylan Thacker-Smith <[email protected]>
  • Loading branch information
shopmike and dylanahsmith authored Sep 18, 2019
1 parent 1dcad34 commit 0d26f05
Show file tree
Hide file tree
Showing 121 changed files with 379 additions and 150 deletions.
7 changes: 0 additions & 7 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,6 @@ Style/DateTime:
Exclude:
- 'test/unit/context_unit_test.rb'

# Offense count: 119
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, never
Style/FrozenStringLiteralComment:
Enabled: false

# Offense count: 9
# Cop supports --auto-correct.
# Configuration parameters: AllowAsExpressionSeparator.
Expand Down
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

source 'https://rubygems.org'
git_source(:github) do |repo_name|
"https://github.com/#{repo_name}.git"
Expand Down
2 changes: 2 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'rake'
require 'rake/testtask'
$LOAD_PATH.unshift(File.expand_path("../lib", __FILE__))
Expand Down
2 changes: 2 additions & 0 deletions example/server/example_servlet.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module ProductsFilter
def price(integer)
format("$%.2d USD", integer / 100.0)
Expand Down
2 changes: 2 additions & 0 deletions example/server/liquid_servlet.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

class LiquidServlet < WEBrick::HTTPServlet::AbstractServlet
def do_GET(req, res)
handle(:get, req, res)
Expand Down
2 changes: 2 additions & 0 deletions example/server/server.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'webrick'
require 'rexml/document'

Expand Down
10 changes: 6 additions & 4 deletions lib/liquid.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Copyright (c) 2005 Tobias Luetke
#
# Permission is hereby granted, free of charge, to any person obtaining
Expand All @@ -21,10 +23,10 @@

module Liquid
FilterSeparator = /\|/
ArgumentSeparator = ','.freeze
FilterArgumentSeparator = ':'.freeze
VariableAttributeSeparator = '.'.freeze
WhitespaceControl = '-'.freeze
ArgumentSeparator = ','
FilterArgumentSeparator = ':'
VariableAttributeSeparator = '.'
WhitespaceControl = '-'
TagStart = /\{\%/
TagEnd = /\%\}/
VariableSignature = /\(?[\w\-\.\[\]]\)?/
Expand Down
16 changes: 9 additions & 7 deletions lib/liquid/block.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Liquid
class Block < Tag
MAX_DEPTH = 100
Expand Down Expand Up @@ -27,16 +29,16 @@ def nodelist
end

def unknown_tag(tag, _params, _tokens)
if tag == 'else'.freeze
raise SyntaxError, parse_context.locale.t("errors.syntax.unexpected_else".freeze,
if tag == 'else'
raise SyntaxError, parse_context.locale.t("errors.syntax.unexpected_else",
block_name: block_name)
elsif tag.start_with?('end'.freeze)
raise SyntaxError, parse_context.locale.t("errors.syntax.invalid_delimiter".freeze,
elsif tag.start_with?('end')
raise SyntaxError, parse_context.locale.t("errors.syntax.invalid_delimiter",
tag: tag,
block_name: block_name,
block_delimiter: block_delimiter)
else
raise SyntaxError, parse_context.locale.t("errors.syntax.unknown_tag".freeze, tag: tag)
raise SyntaxError, parse_context.locale.t("errors.syntax.unknown_tag", tag: tag)
end
end

Expand All @@ -52,7 +54,7 @@ def block_delimiter

def parse_body(body, tokens)
if parse_context.depth >= MAX_DEPTH
raise StackLevelError, "Nesting too deep".freeze
raise StackLevelError, "Nesting too deep"
end
parse_context.depth += 1
begin
Expand All @@ -61,7 +63,7 @@ def parse_body(body, tokens)

return false if end_tag_name == block_delimiter
unless end_tag_name
raise SyntaxError, parse_context.locale.t("errors.syntax.tag_never_closed".freeze, block_name: block_name)
raise SyntaxError, parse_context.locale.t("errors.syntax.tag_never_closed", block_name: block_name)
end

# this tag is not registered with the system
Expand Down
20 changes: 11 additions & 9 deletions lib/liquid/block_body.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
# frozen_string_literal: true

module Liquid
class BlockBody
LiquidTagToken = /\A\s*(\w+)\s*(.*?)\z/o
FullToken = /\A#{TagStart}#{WhitespaceControl}?(\s*)(\w+)(\s*)(.*?)#{WhitespaceControl}?#{TagEnd}\z/om
ContentOfVariable = /\A#{VariableStart}#{WhitespaceControl}?(.*?)#{WhitespaceControl}?#{VariableEnd}\z/om
WhitespaceOrNothing = /\A\s*\z/
TAGSTART = "{%".freeze
VARSTART = "{{".freeze
TAGSTART = "{%"
VARSTART = "{{"

attr_reader :nodelist

Expand Down Expand Up @@ -64,10 +66,10 @@ def parse(tokenizer, parse_context, &block)
if parse_context.line_number
# newlines inside the tag should increase the line number,
# particularly important for multiline {% liquid %} tags
parse_context.line_number += Regexp.last_match(1).count("\n".freeze) + Regexp.last_match(3).count("\n".freeze)
parse_context.line_number += Regexp.last_match(1).count("\n") + Regexp.last_match(3).count("\n")
end

if tag_name == 'liquid'.freeze
if tag_name == 'liquid'
liquid_tag_tokenizer = Tokenizer.new(markup, line_number: parse_context.line_number, for_liquid_tag: true)
next parse_for_liquid_tag(liquid_tag_tokenizer, parse_context, &block)
end
Expand Down Expand Up @@ -113,7 +115,7 @@ def blank?
end

def render(context)
render_to_output_buffer(context, '')
render_to_output_buffer(context, +'')
end

def render_to_output_buffer(context, output)
Expand All @@ -129,7 +131,7 @@ def render_to_output_buffer(context, output)
when Variable
render_node(context, output, node)
when Block
render_node(context, node.blank? ? '' : output, node)
render_node(context, node.blank? ? +'' : output, node)
break if context.interrupt? # might have happened in a for-block
when Continue, Break
# If we get an Interrupt that means the block must stop processing. An
Expand Down Expand Up @@ -163,7 +165,7 @@ def render_node(context, output, node)
def raise_if_resource_limits_reached(context, length)
context.resource_limits.render_length += length
return unless context.resource_limits.reached?
raise MemoryError, "Memory limits exceeded".freeze
raise MemoryError, "Memory limits exceeded"
end

def create_variable(token, parse_context)
Expand All @@ -175,11 +177,11 @@ def create_variable(token, parse_context)
end

def raise_missing_tag_terminator(token, parse_context)
raise SyntaxError, parse_context.locale.t("errors.syntax.tag_termination".freeze, token: token, tag_end: TagEnd.inspect)
raise SyntaxError, parse_context.locale.t("errors.syntax.tag_termination", token: token, tag_end: TagEnd.inspect)
end

def raise_missing_variable_terminator(token, parse_context)
raise SyntaxError, parse_context.locale.t("errors.syntax.variable_termination".freeze, token: token, tag_end: VariableEnd.inspect)
raise SyntaxError, parse_context.locale.t("errors.syntax.variable_termination", token: token, tag_end: VariableEnd.inspect)
end

def registered_tags
Expand Down
20 changes: 11 additions & 9 deletions lib/liquid/condition.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Liquid
# Container for liquid nodes which conveniently wraps decision making logic
#
Expand All @@ -8,14 +10,14 @@ module Liquid
#
class Condition #:nodoc:
@@operators = {
'=='.freeze => ->(cond, left, right) { cond.send(:equal_variables, left, right) },
'!='.freeze => ->(cond, left, right) { !cond.send(:equal_variables, left, right) },
'<>'.freeze => ->(cond, left, right) { !cond.send(:equal_variables, left, right) },
'<'.freeze => :<,
'>'.freeze => :>,
'>='.freeze => :>=,
'<='.freeze => :<=,
'contains'.freeze => lambda do |_cond, left, right|
'==' => ->(cond, left, right) { cond.send(:equal_variables, left, right) },
'!=' => ->(cond, left, right) { !cond.send(:equal_variables, left, right) },
'<>' => ->(cond, left, right) { !cond.send(:equal_variables, left, right) },
'<' => :<,
'>' => :>,
'>=' => :>=,
'<=' => :<=,
'contains' => lambda do |_cond, left, right|
if left && right && left.respond_to?(:include?)
right = right.to_s if left.is_a?(String)
left.include?(right)
Expand Down Expand Up @@ -78,7 +80,7 @@ def else?
end

def inspect
"#<Condition #{[@left, @operator, @right].compact.join(' '.freeze)}>"
"#<Condition #{[@left, @operator, @right].compact.join(' ')}>"
end

protected
Expand Down
4 changes: 3 additions & 1 deletion lib/liquid/context.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Liquid
# Context keeps the variable stack and resolves variables, as well as keywords
#
Expand Down Expand Up @@ -232,7 +234,7 @@ def try_variable_find_in_environments(key, raise_on_not_found:)
end

def check_overflow
raise StackLevelError, "Nesting too deep".freeze if overflow?
raise StackLevelError, "Nesting too deep" if overflow?
end

def overflow?
Expand Down
8 changes: 5 additions & 3 deletions lib/liquid/document.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Liquid
class Document < BlockBody
def self.parse(tokens, parse_context)
Expand All @@ -17,10 +19,10 @@ def parse(tokens, parse_context)

def unknown_tag(tag, parse_context)
case tag
when 'else'.freeze, 'end'.freeze
raise SyntaxError, parse_context.locale.t("errors.syntax.unexpected_outer_tag".freeze, tag: tag)
when 'else', 'end'
raise SyntaxError, parse_context.locale.t("errors.syntax.unexpected_outer_tag", tag: tag)
else
raise SyntaxError, parse_context.locale.t("errors.syntax.unknown_tag".freeze, tag: tag)
raise SyntaxError, parse_context.locale.t("errors.syntax.unknown_tag", tag: tag)
end
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/liquid/drop.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'set'

module Liquid
Expand Down
6 changes: 4 additions & 2 deletions lib/liquid/errors.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
# frozen_string_literal: true

module Liquid
class Error < ::StandardError
attr_accessor :line_number
attr_accessor :template_name
attr_accessor :markup_context

def to_s(with_prefix = true)
str = ""
str = +""
str << message_prefix if with_prefix
str << super()

Expand All @@ -20,7 +22,7 @@ def to_s(with_prefix = true)
private

def message_prefix
str = ""
str = +""
str << if is_a?(SyntaxError)
"Liquid syntax error"
else
Expand Down
12 changes: 7 additions & 5 deletions lib/liquid/expression.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Liquid
class Expression
class MethodLiteral
Expand All @@ -14,11 +16,11 @@ def to_liquid
end

LITERALS = {
nil => nil, 'nil'.freeze => nil, 'null'.freeze => nil, ''.freeze => nil,
'true'.freeze => true,
'false'.freeze => false,
'blank'.freeze => MethodLiteral.new(:blank?, '').freeze,
'empty'.freeze => MethodLiteral.new(:empty?, '').freeze
nil => nil, 'nil' => nil, 'null' => nil, '' => nil,
'true' => true,
'false' => false,
'blank' => MethodLiteral.new(:blank?, '').freeze,
'empty' => MethodLiteral.new(:empty?, '').freeze
}.freeze

SINGLE_QUOTED_STRING = /\A'(.*)'\z/m
Expand Down
2 changes: 2 additions & 0 deletions lib/liquid/extensions.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'time'
require 'date'

Expand Down
6 changes: 4 additions & 2 deletions lib/liquid/file_system.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Liquid
# A Liquid file system is a way to let your templates retrieve other templates for use with the include tag.
#
Expand Down Expand Up @@ -44,7 +46,7 @@ def read_template_file(_template_path)
class LocalFileSystem
attr_accessor :root

def initialize(root, pattern = "_%s.liquid".freeze)
def initialize(root, pattern = "_%s.liquid")
@root = root
@pattern = pattern
end
Expand All @@ -59,7 +61,7 @@ def read_template_file(template_path)
def full_path(template_path)
raise FileSystemError, "Illegal template name '#{template_path}'" unless template_path =~ %r{\A[^./][a-zA-Z0-9_/]+\z}

full_path = if template_path.include?('/'.freeze)
full_path = if template_path.include?('/')
File.join(root, File.dirname(template_path), @pattern % File.basename(template_path))
else
File.join(root, @pattern % template_path)
Expand Down
2 changes: 2 additions & 0 deletions lib/liquid/forloop_drop.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Liquid
class ForloopDrop < Drop
def initialize(name, length, parentloop)
Expand Down
4 changes: 3 additions & 1 deletion lib/liquid/i18n.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'yaml'

module Liquid
Expand Down Expand Up @@ -31,7 +33,7 @@ def interpolate(name, vars)
end

def deep_fetch_translation(name)
name.split('.'.freeze).reduce(locale) do |level, cur|
name.split('.').reduce(locale) do |level, cur|
level[cur] || raise(TranslationError, "Translation for #{name} does not exist in locale #{path}")
end
end
Expand Down
4 changes: 3 additions & 1 deletion lib/liquid/interrupts.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
# frozen_string_literal: true

module Liquid
# An interrupt is any command that breaks processing of a block (ex: a for loop).
class Interrupt
attr_reader :message

def initialize(message = nil)
@message = message || "interrupt".freeze
@message = message || "interrupt"
end
end

Expand Down
Loading

0 comments on commit 0d26f05

Please sign in to comment.