Skip to content

Commit

Permalink
Add strict locals linter
Browse files Browse the repository at this point in the history
  • Loading branch information
simonlevasseur committed Jan 20, 2025
1 parent 1cba325 commit 61c68fe
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 2 deletions.
26 changes: 24 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ linters:
| [FinalNewline](#finalnewline) | Yes | warns about missing newline at the end of a ERB template |
| [NoJavascriptTagHelper](#nojavascripttaghelper) | Yes | prevents the usage of Rails' `javascript_tag` |
| ParserErrors | Yes | |
| PartialInstanceVariable | No | detects instance variables in partials |
| [RequireInputAutocomplete](#requireinputautocomplete) | Yes | warns about missing autocomplete attributes in input tags |
| [RightTrim](#righttrim) | Yes | enforces trimming at the right of an ERB tag |
| [SelfClosingTag](#selfclosingtag) | Yes | enforces self closing tag styles for void elements |
Expand All @@ -121,9 +120,11 @@ linters:
| TrailingWhitespace | Yes | |
| [DeprecatedClasses](#deprecatedclasses) | No | warns about deprecated css classes |
| [ErbSafety](#erbsafety) | No | detects unsafe interpolation of ruby data into various javascript contexts and enforce usage of safe helpers like `.to_json`. |
| [HardCodedString](#hardcodedstring) | No | warns if there is a visible hardcoded string in the DOM (does not check for a hardcoded string nested inside a JavaScript tag) |
| PartialInstanceVariable | No | detects instance variables in partials |
| [Rubocop](#rubocop) | No | runs RuboCop rules on ruby statements found in ERB templates |
| [RequireScriptNonce](#requirescriptnonce) | No | warns about missing [Content Security Policy nonces](https://guides.rubyonrails.org/security.html#content-security-policy) in script tags |
| [HardCodedString](#hardcodedstring) | No | warns if there is a visible hardcoded string in the DOM (does not check for a hardcoded string nested inside a JavaScript tag) |
| [StrictLocals](#strictlocals) | No | enforces the use of strict locals in Rails view partial templates |

### DeprecatedClasses

Expand Down Expand Up @@ -544,6 +545,27 @@ class I18nCorrector
end
```

### StrictLocals
This linter enforces the use of [strict locals](https://guides.rubyonrails.org/layouts_and_rendering.html#strict-locals) in Rails view partial templates.

```
# app/views/foo/_bar.html.erb

Bad ❌
<div>
My name is <%= @name %>
</div>

Good ✅
<%# locals: (name:) %>
<div>
My name is <%= name %>
</div>
```
**Note**: This linter does not enforce the use of strict locals in view templates (files that don't start with `_`).
**Note**: This linter does not prevent the use of instance variables. It merely enforces that a strict locals declaration is present. It's recommended to use this linter in conjunction with the `PartialInstanceVariable` linter to enforce the use of locals.
## CommentSyntax
This linter enforces the use of the correct ERB comment syntax, since Ruby comments (`<% # comment %>` with a space) are not technically valid ERB comments.
Expand Down
50 changes: 50 additions & 0 deletions lib/erb_lint/linters/strict_locals.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# frozen_string_literal: true

module ERBLint
module Linters
# Enforces the use of strict locals in Rails view partial templates.
class StrictLocals < Linter
include LinterRegistry

STRICT_LOCALS_REGEX = /\s+locals:\s+\((.*)\)/

def initialize(file_loader, config)
super
end

def run(processed_source)
return unless processed_source.filename.match?(%r{(\A|.*/)_[^/\s]*\.html\.erb\z})

file_content = processed_source.file_content
return if file_content.empty?

strict_locals_node = processed_source.ast.descendants(:erb).find do |erb_node|
indicator_node, _, code_node, _ = *erb_node

indicator_node_str = indicator_node&.deconstruct&.last
next unless indicator_node_str == "#"

code_node_str = code_node&.deconstruct&.last

code_node_str.match(STRICT_LOCALS_REGEX)
end

unless strict_locals_node
add_offense(
processed_source.to_source_range(0...processed_source.file_content.size),
<<~EOF.chomp,
Missing strict locals declaration.
Add <%# locals: () %> at the top of the file to enforce strict locals.
EOF
)
end
end

def autocorrect(_processed_source, offense)
lambda do |corrector|
corrector.insert_before(offense.source_range, "<%# locals: () %>\n")
end
end
end
end
end
114 changes: 114 additions & 0 deletions spec/erb_lint/linters/strict_locals_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# frozen_string_literal: true

require "spec_helper"
require "debug"

describe ERBLint::Linters::StrictLocals do
let(:linter_config) { described_class.config_schema.new }
let(:file_loader) { ERBLint::FileLoader.new(".") }
let(:linter) { described_class.new(file_loader, linter_config) }

subject { linter.offenses }
before { linter.run(processed_source) }

context "when the ERB is not a view partial" do
let(:file) { <<~FILE }
<div>
<%= foo %>
</div>
FILE

context "when the ERB is a simple file" do
let(:processed_source) { ERBLint::ProcessedSource.new("file.html.erb", file) }

it "does not report any offenses" do
expect(subject).to(be_empty)
end
end

context "when the ERB is a nested file" do
let(:processed_source) { ERBLint::ProcessedSource.new("foo/bar/baz/my_file.html.erb", file) }

it "does not report any offenses" do
expect(subject).to(be_empty)
end
end
end

context "when the ERB is empty" do
let(:file) { "" }
let(:processed_source) { ERBLint::ProcessedSource.new("_file.html.erb", file) }

it "does not report any offenses" do
expect(subject).to(be_empty)
end
end

context "when the ERB is a view partial" do
let(:processed_source) { ERBLint::ProcessedSource.new("_file.html.erb", file) }

context "when the ERB contains a strict locals declaration at the top of the file" do
let(:file) { <<~FILE }
<%# locals: (foo: "bar") %>
<div>
<%= foo %>
</div>
FILE

it "does not report any offenses" do
expect(subject).to(be_empty)
end
end

context "when the ERB contains a strict locals declaration anywhere else in the file" do
let(:file) { <<~FILE }
<div>
<%= foo %>
</div>
<%# locals: (foo: "bar") %>
FILE

it "does not report any offenses" do
expect(subject).to(be_empty)
end
end

context "when the ERB contains an empty strict locals declaration" do
let(:file) { <<~FILE }
<%# locals: () %>
<div>
<%= foo %>
</div>
FILE

it "does not report any offenses" do
expect(subject).to(be_empty)
end
end

context "when the ERB does not contain a strict locals declaration" do
let(:file) { <<~FILE }
<div>
<%= foo %>
</div>
FILE
let(:corrector) { ERBLint::Corrector.new(processed_source, subject) }
let(:corrected_content) { corrector.corrected_content }

it "reports an offense" do
expect(subject.size).to(eq(1))
end

it "reports the suggested fix" do
expect(subject.first.message).to(include(
"Missing strict locals declaration.\n",
"Add <%# locals: () %> at the top of the file to enforce strict locals.",
))
end

it "corrects the file" do
expect(corrected_content).to(eq("<%# locals: () %>\n<div>\n <%= foo %>\n</div>\n"))
end
end
end
end

0 comments on commit 61c68fe

Please sign in to comment.