diff --git a/README.md b/README.md index a1ec280..3145c82 100644 --- a/README.md +++ b/README.md @@ -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 | @@ -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 @@ -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 ❌ +
+ My name is <%= @name %> +
+ +Good ✅ +<%# locals: (name:) %> +
+ My name is <%= name %> +
+``` + +**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. diff --git a/lib/erb_lint/linters/strict_locals.rb b/lib/erb_lint/linters/strict_locals.rb new file mode 100644 index 0000000..f533a8f --- /dev/null +++ b/lib/erb_lint/linters/strict_locals.rb @@ -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 diff --git a/spec/erb_lint/linters/strict_locals_spec.rb b/spec/erb_lint/linters/strict_locals_spec.rb new file mode 100644 index 0000000..20f0a1c --- /dev/null +++ b/spec/erb_lint/linters/strict_locals_spec.rb @@ -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 } +
+ <%= foo %> +
+ 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") %> +
+ <%= foo %> +
+ 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 } +
+ <%= foo %> +
+ <%# 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: () %> +
+ <%= foo %> +
+ 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 } +
+ <%= foo %> +
+ 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
\n <%= foo %>\n
\n")) + end + end + end +end