Skip to content

Commit 5a0e72b

Browse files
committed
Fix undeclared lax-dependent specs + add verify_* lints
20 specs parse-fail in the default mode (strict2) but produce their expected output under lax, yet declared no error_mode. They were functionally broken: they failed for every adapter (ran in strict2) and weren't auto-tagged lax_parsing, so lax-opt-out adapters couldn't skip them either. Add `error_mode: lax` to each (12 in liquid_ruby/specs.yml, 8 in shopify_production_recordings/recorded_specs.yml). The gem auto-tags them lax_parsing, so non-lax adapters now skip them and lax-enabled adapters run them in lax mode. New lints: - scripts/verify_lax_mode_declared.rb: flags specs that need lax mode but declare no error_mode (parse-fails strict2, matches in lax, no errors). Push gate. Green. - scripts/verify_lax_placement.rb: repaired stale API (Suite#specs -> SpecLoader.load_suite; spec.environment -> instantiate_environment, etc.). Semantic audit: reports lax-only specs not in the liquid_ruby_lax suite (35 known-debt; move with scripts/move_spec.rb). AGENTS.md: document the verify_* push gate vs. semantic audit split and the error-mode policy (needs lax -> declare error_mode: lax; lax-only specs belong in liquid_ruby_lax; lax-vs-strict2 differences declare error_mode: strict2).
1 parent 2dc2294 commit 5a0e72b

5 files changed

Lines changed: 182 additions & 16 deletions

File tree

CLAUDE.md

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,23 +57,37 @@ If it fails, add a useful spec/source-level hint or move the spec later in the r
5757
### `verify_*` scripts
5858

5959
`scripts/` contains standalone lint/audit scripts named `verify_*.rb` that check
60-
cross-cutting spec invariants the quality-gate test doesn't cover:
60+
cross-cutting spec invariants the quality-gate test doesn't cover.
61+
62+
**Push gate** (mechanical, must be green before pushing):
6163

6264
```bash
63-
ruby -Ilib scripts/verify_lax_placement.rb # lax-only specs live in liquid_ruby_lax
6465
ruby -Ilib scripts/verify_ruby_type_tags.rb # Ruby-content specs carry a ruby feature tag + complexity > 100
66+
ruby -Ilib scripts/verify_lax_mode_declared.rb # lax-dependent specs declare error_mode: lax (auto-tags lax_parsing)
6567
```
6668

67-
**Run every `verify_*` script before pushing.** They exit non-zero on violation:
68-
6969
```bash
70-
for s in scripts/verify_*.rb; do ruby -Ilib "$s" || exit 1; done
70+
for s in scripts/verify_ruby_type_tags.rb scripts/verify_lax_mode_declared.rb; do ruby -Ilib "$s" || exit 1; done
7171
```
7272

73-
When you introduce a new cross-cutting rule (e.g. "every spec with marker X must
74-
have tag Y"), add a `verify_*.rb` script for it rather than a one-off check, so
75-
the rule stays enforced. Keep each script self-contained (parse spec files
76-
directly), print `OK: ...` on success, and exit non-zero with a per-spec
73+
**Semantic audit** (slower; runs specs against reference liquid in both modes;
74+
report known debt, don't block push):
75+
76+
```bash
77+
ruby -Ilib scripts/verify_lax_placement.rb # lax-only specs should live in the liquid_ruby_lax suite
78+
```
79+
Error-mode policy these enforce:
80+
- A spec that needs lax mode must declare `error_mode: lax` (the gem auto-tags
81+
it `lax_parsing`, so lax-opt-out adapters skip it). `verify_lax_mode_declared`
82+
catches any that forgot.
83+
- Lax-only specs belong in `specs/liquid_ruby_lax/`, not `specs/liquid_ruby/`.
84+
`verify_lax_placement` reports misplaced ones; move them with `scripts/move_spec.rb`.
85+
- Specs exercising a lax-vs-strict2 difference that matters declare
86+
`error_mode: strict2` (auto-tagged `strict2_parsing`).
87+
88+
When you introduce a new cross-cutting rule, add a `verify_*.rb` script for it
89+
rather than a one-off check, so the rule stays enforced. Keep each script
90+
self-contained, print `OK: ...` on success, and exit non-zero with a per-spec
7791
offender list on failure.
7892

7993
### Dumb Adapter Ramp Audits
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
#!/usr/bin/env ruby
2+
# frozen_string_literal: true
3+
4+
# Lint: specs that need lax mode must declare `error_mode: lax`.
5+
#
6+
# A spec is "lax-dependent" if it parse-fails in the default mode (strict2) but
7+
# produces its expected output under lax. If it doesn't declare `error_mode: lax`
8+
# (and doesn't expect a parse error via `errors:`), it is functionally broken:
9+
# it runs in strict2 and fails for EVERY adapter, and it isn't auto-tagged
10+
# `lax_parsing` so lax-opt-out adapters can't skip it either.
11+
#
12+
# This lint requires the reference `liquid` gem to parse/render templates.
13+
#
14+
# Usage: ruby -Ilib scripts/verify_lax_mode_declared.rb
15+
# Exit non-zero if any violation is found.
16+
17+
require "yaml"
18+
require "liquid"
19+
20+
module LaxModeDeclaredVerifier
21+
class << self
22+
def run
23+
offenders = []
24+
each_spec do |spec|
25+
next if spec[:lax_applied] # file/suite already applies lax globally
26+
next if spec[:error_mode] # declares a mode already
27+
next if expects_parse_error?(spec) # legitimately expects parse failure
28+
tmpl = spec[:template].to_s
29+
expected = spec[:expected]
30+
next if expected.nil? # no expected output to compare
31+
begin
32+
Liquid::Template.parse(tmpl, error_mode: :strict2)
33+
next # parses in strict2 -> not lax-dependent
34+
rescue Liquid::SyntaxError, StandardError
35+
# strict2 rejects it -> check lax
36+
end
37+
begin
38+
actual = Liquid::Template.parse(tmpl, error_mode: :lax).render
39+
next unless actual == expected # lax doesn't match -> different problem
40+
rescue StandardError
41+
next # lax also fails -> different problem
42+
end
43+
offenders << spec
44+
end
45+
46+
if offenders.empty?
47+
puts "OK: all lax-dependent specs declare error_mode: lax."
48+
return 0
49+
end
50+
51+
puts "Found #{offenders.size} lax-dependent spec(s) missing `error_mode: lax`:\n\n"
52+
offenders.each do |o|
53+
puts " #{o[:file]}:#{o[:line]} #{o[:name]}"
54+
puts " #{o[:template].to_s.gsub(/\n/, " ")[0, 80]}"
55+
puts " fix: add `error_mode: lax` (auto-tags lax_parsing)\n\n"
56+
end
57+
1
58+
end
59+
60+
private
61+
62+
def expects_parse_error?(spec)
63+
errs = spec[:errors]
64+
return false unless errs.is_a?(Hash)
65+
errs.key?("parse_error") || errs.key?(:parse_error)
66+
end
67+
68+
def each_spec
69+
Dir.glob("specs/**/*.yml").sort.each do |file|
70+
doc = YAML.unsafe_load(File.read(file)) rescue next
71+
specs = specs_of(doc)
72+
next unless specs.is_a?(Array)
73+
starts = index_block_starts(file)
74+
lax_applied = file_applies_lax?(file, doc)
75+
specs.each do |s|
76+
next unless s.is_a?(Hash)
77+
yield(
78+
file: file,
79+
line: starts[s["name"].to_s],
80+
name: s["name"],
81+
template: s["template"],
82+
expected: s["expected"],
83+
error_mode: s["error_mode"],
84+
errors: s["errors"],
85+
lax_applied: lax_applied,
86+
)
87+
end
88+
end
89+
end
90+
91+
# True if this file (or its suite) applies error_mode: lax globally, so
92+
# individual specs need not declare it.
93+
def file_applies_lax?(file, doc)
94+
meta = doc.is_a?(Hash) ? doc["_metadata"] : nil
95+
req = meta.is_a?(Hash) ? (meta["required_options"] || {}) : {}
96+
return true if req["error_mode"].to_s == "lax"
97+
suite_yml = File.join(File.dirname(file), "suite.yml")
98+
if File.file?(suite_yml)
99+
s = YAML.unsafe_load(File.read(suite_yml)) rescue nil
100+
defaults = s.is_a?(Hash) ? (s["defaults"] || {}) : {}
101+
return true if defaults["error_mode"].to_s == "lax"
102+
end
103+
false
104+
end
105+
106+
def specs_of(d)
107+
return d if d.is_a?(Array)
108+
return d["specs"] || d["tests"] if d.is_a?(Hash)
109+
nil
110+
end
111+
112+
def index_block_starts(file)
113+
map = {}
114+
lines = File.readlines(file)
115+
starts = lines.each_index.select { |i| lines[i] =~ /^- [A-Za-z]/ }
116+
starts.each_with_index do |st, k|
117+
en = (k + 1 < starts.size) ? starts[k + 1] : lines.size
118+
(st...en).each do |i|
119+
if lines[i] =~ /^ name:\s*(.+?)\s*$/
120+
map[$1] = st + 1 unless map.key?($1)
121+
break
122+
end
123+
end
124+
end
125+
map
126+
end
127+
end
128+
end
129+
130+
exit LaxModeDeclaredVerifier.run if $PROGRAM_NAME == __FILE__

scripts/verify_lax_placement.rb

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
require "liquid"
1313
require "liquid/spec"
1414
require "liquid/spec/suite"
15+
require "liquid/spec/spec_loader"
1516
require "liquid/spec/deps/liquid_ruby"
1617
require "liquid/spec/yaml_initializer"
1718
require "timecop"
@@ -39,8 +40,8 @@ def verify_all_specs
3940
end
4041

4142
# Get specs from each suite
42-
strict_suite_specs = liquid_ruby_suite.specs
43-
lax_suite_specs = liquid_ruby_lax_suite.specs
43+
strict_suite_specs = Liquid::Spec::SpecLoader.load_suite(liquid_ruby_suite)
44+
lax_suite_specs = Liquid::Spec::SpecLoader.load_suite(liquid_ruby_lax_suite)
4445

4546
# Track spec names in lax suite for quick lookup
4647
lax_suite_names = lax_suite_specs.map(&:name).to_set
@@ -176,7 +177,7 @@ def test_spec_with_mode(spec, mode)
176177
end
177178

178179
# Build assigns
179-
assigns = deep_copy(spec.environment || {})
180+
assigns = deep_copy(spec.instantiate_environment)
180181

181182
# Build context with static_environments (same as liquid_ruby adapter)
182183
# This is important for increment/decrement tests which use separate namespaces
@@ -201,18 +202,19 @@ def test_spec_with_mode(spec, mode)
201202
def build_registers(spec)
202203
registers = {}
203204
registers[:file_system] = build_file_system(spec)
204-
registers[:template_factory] = spec.template_factory if spec.template_factory
205+
registers[:template_factory] = spec.instantiate_template_factory if spec.raw_template_factory
205206
registers
206207
end
207208

208209
def build_file_system(spec)
209-
case spec.filesystem
210+
fs = spec.instantiate_filesystem
211+
case fs
210212
when Hash
211-
StubFileSystem.new(spec.filesystem)
213+
StubFileSystem.new(fs)
212214
when nil
213215
Liquid::BlankFileSystem.new
214216
else
215-
spec.filesystem
217+
fs
216218
end
217219
end
218220

specs/liquid_ruby/specs.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ specs:
273273
expected: Print this-thing
274274
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/integration/capture_test.rb#L17
275275
- name: CaptureTest#test_captures_block_content_in_variable_a587b04c
276+
error_mode: lax
276277
template: "{% capture 'var' %}test string{% endcapture %}{{var}}"
277278
render_errors: false
278279
complexity: 300
@@ -669,6 +670,7 @@ specs:
669670
expected: one
670671
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/unit/tags/case_tag_unit_test.rb#L45
671672
- name: CaseTagUnitTest#test_case_when_with_trailing_element_3597f047
673+
error_mode: lax
672674
template: |
673675
{%- case 1 -%}
674676
{%- when 1 bar -%}
@@ -725,6 +727,7 @@ specs:
725727
expected: one
726728
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/unit/tags/case_tag_unit_test.rb#L24
727729
- name: CaseTagUnitTest#test_case_with_trailing_element_53df2d40
730+
error_mode: lax
728731
template: |
729732
{%- case 1 bar -%}
730733
{%- when 1 -%}
@@ -1482,12 +1485,14 @@ specs:
14821485
expected: '112233'
14831486
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/integration/tags/cycle_tag_test.rb#L50
14841487
- name: CycleTagTest#test_cycle_tag_with_error_mode_2e06b45a
1488+
error_mode: lax
14851489
template: "{% assign 5 = 'b' %}{% cycle .5, .4 %}"
14861490
render_errors: false
14871491
complexity: 150
14881492
expected: b
14891493
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/integration/tags/cycle_tag_test.rb#L100
14901494
- name: CycleTagTest#test_cycle_tag_with_error_mode_577d33f5
1495+
error_mode: lax
14911496
template: "{% cycle .5: 'a', 'b' %}"
14921497
render_errors: false
14931498
complexity: 150
@@ -1508,6 +1513,7 @@ specs:
15081513
expected: b
15091514
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/integration/tags/cycle_tag_test.rb#L100
15101515
- name: CycleTagTest#test_cycle_with_trailing_elements_24de1b7f
1516+
error_mode: lax
15111517
template: "{% assign a = 'A' %}{% assign n = 'N' %}{% cycle n e: 'a', 'b', 'c'
15121518
%}"
15131519
render_errors: false
@@ -1523,6 +1529,7 @@ specs:
15231529
expected: a
15241530
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/integration/tags/cycle_tag_test.rb#L125
15251531
- name: CycleTagTest#test_cycle_with_trailing_elements_5b8114bf
1532+
error_mode: lax
15261533
template: "{% assign a = 'A' %}{% assign n = 'N' %}{% cycle n e 'a', 'b', 'c'
15271534
%}"
15281535
render_errors: false
@@ -1538,13 +1545,15 @@ specs:
15381545
expected: a
15391546
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/integration/tags/cycle_tag_test.rb#L126
15401547
- name: CycleTagTest#test_cycle_with_trailing_elements_692ee629
1548+
error_mode: lax
15411549
template: "{% assign a = 'A' %}{% assign n = 'N' %}{% cycle 'a' 'b', 'c'
15421550
%}"
15431551
render_errors: false
15441552
complexity: 150
15451553
expected: a
15461554
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/integration/tags/cycle_tag_test.rb#L125
15471555
- name: CycleTagTest#test_cycle_with_trailing_elements_6d4832a1
1556+
error_mode: lax
15481557
template: "{% assign a = 'A' %}{% assign n = 'N' %}{% cycle name: 'a' 'b', 'c'
15491558
%}"
15501559
render_errors: false
@@ -1568,6 +1577,7 @@ specs:
15681577
expected: "N"
15691578
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/integration/tags/cycle_tag_test.rb#L129
15701579
- name: CycleTagTest#test_cycle_with_trailing_elements_c853a037
1580+
error_mode: lax
15711581
template: "{% assign a = 'A' %}{% assign n = 'N' %}{% cycle name: 'a', 'b' 'c'
15721582
%}"
15731583
render_errors: false
@@ -7656,6 +7666,7 @@ specs:
76567666
<td class="col1">1</td><td class="col2">2</td><td class="col3">3</td><td class="col4">4</td><td class="col5">5</td></tr>
76577667
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/integration/tags/table_row_test.rb#L443
76587668
- name: TableRowTest#test_tablerow_with_invalid_attribute_strict_vs_strict2_b293b0fe
7669+
error_mode: lax
76597670
template: "{% tablerow i in (1..5) invalid_attr: 10 %}{{ i }}{% endtablerow %}"
76607671
render_errors: false
76617672
complexity: 180
@@ -7664,6 +7675,7 @@ specs:
76647675
<td class="col1">1</td><td class="col2">2</td><td class="col3">3</td><td class="col4">4</td><td class="col5">5</td></tr>
76657676
url: https://github.com/Shopify/liquid/blob/59d8d0d22db3b4865deecde0ab2683bea25a1e9f/test/integration/tags/table_row_test.rb#L443
76667677
- name: TableRowTest#test_tablerow_with_invalid_expression_strict_vs_strict2_944ef6e0
7678+
error_mode: lax
76677679
template: "{% tablerow i in (1..5) limit: foo=>bar %}{{ i }}{% endtablerow %}"
76687680
render_errors: false
76697681
complexity: 180

specs/shopify_production_recordings/recorded_specs.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2182,6 +2182,7 @@
21822182
environment: {}
21832183
filesystem: {}
21842184
- name: RecordedTest#test_conditional_tag_returns_false_when_the_operator_is_unknown_5c21eaec8
2185+
error_mode: lax
21852186
features:
21862187
- lax_parsing
21872188
expected: ''
@@ -2205,13 +2206,15 @@
22052206
template: "{% if true true %}{% else %}1{% endif %}"
22062207
environment: {}
22072208
- name: RecordedTest#test_does_not_output_error_when_short_circuit_evaluation_avoids_using_the_unknown_operator_68463361a
2209+
error_mode: lax
22082210
features:
22092211
- lax_parsing
22102212
expected: b
22112213
template: "{% if dynamic and true true %}a{% else %}b{% endif %}"
22122214
environment:
22132215
dynamic: false
22142216
- name: RecordedTest#test_does_not_output_error_when_unknown_operators_are_used_in_elsif_but_if_is_true_9f923dacb
2217+
error_mode: lax
22152218
features:
22162219
- lax_parsing
22172220
expected: a
@@ -2254,6 +2257,7 @@
22542257
environment: {}
22552258
filesystem: {}
22562259
- name: RecordedTest#test_ignore_unknown_operators_when_the_tags_body_is_empty_c0bf47389
2260+
error_mode: lax
22572261
features:
22582262
- lax_parsing
22592263
expected: ''
@@ -3690,6 +3694,7 @@
36903694
environment: {}
36913695
filesystem: {}
36923696
- name: RecordedTest#test_break_in_capture_in_loop_12d168c32
3697+
error_mode: lax
36933698
expected: 'before,after,captured:inside
36943699

36953700
'
@@ -3709,6 +3714,7 @@
37093714
environment: {}
37103715
filesystem: {}
37113716
- name: RecordedTest#test_break_in_capture_not_in_loop_0e9a05bee
3717+
error_mode: lax
37123718
expected: before,
37133719
template: |
37143720
{% liquid
@@ -4488,6 +4494,7 @@
44884494
b: meow
44894495
filesystem: {}
44904496
- name: RecordedTest#test_continue_in_capture_in_loop_1a17a2daf
4497+
error_mode: lax
44914498
expected: 'before,after,captured:inside
44924499

44934500
'
@@ -4512,6 +4519,7 @@
45124519
environment: {}
45134520
filesystem: {}
45144521
- name: RecordedTest#test_data_escaping_9e876fe0d
4522+
error_mode: lax
45154523
features:
45164524
- lax_parsing
45174525
expected: "\n\r\t'\"\\\U0001F44B\U0001F3FB"

0 commit comments

Comments
 (0)