Skip to content

Commit

Permalink
Add InstanceVariables linter (#174)
Browse files Browse the repository at this point in the history
Hello! I would like to add a linter to detect instance variables in Slim
templates. Replacing them with local variables is often encouraged in
Rails partial templates, but here I expect using the `include`
configuration option to define the targeted files so this linter can be
used together with not only partial templates but any glob pattern you
like.

The linter itself uses the work done on the Rubocop linter - namely the
Ruby code extractor. This linter parses the extracted ruby code and
searches for instance variables in there. It also uses the extracted
code source map to report proper line numbers.


![image](https://github.com/sds/slim-lint/assets/462701/6deb617c-5847-4087-8b96-7f3879e6958f)

This closes #155. Thanks!
  • Loading branch information
borama committed Apr 1, 2024
1 parent a6e40ab commit 77affdc
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 0 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ Gemfile.lock
/coverage
/pkg
.ruby-version
/.vscode
6 changes: 6 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ linters:
enabled: false
max: 300

InstanceVariables:
enabled: false
include:
# Include only partial templates by default
- app/views/**/_*.html.slim

LineLength:
enabled: true
max: 80
Expand Down
39 changes: 39 additions & 0 deletions lib/slim_lint/linter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Below is a list of linters supported by `slim-lint`, ordered alphabetically.
* [EmptyControlStatement](#emptycontrolstatement)
* [EmptyLines](#emptylines)
* [FileLength](#filelength)
* [InstanceVariables](#instancevariables)
* [LineLength](#linelength)
* [RedundantDiv](#redundantdiv)
* [RuboCop](#rubocop)
Expand Down Expand Up @@ -157,6 +158,44 @@ linters:

Long files are harder to read and usually indicative of complexity.

## InstanceVariables

Reports instance variables in Slim templates. Use the `include` configuration
option to narrow down the files to e.g. only partial view templates in Rails:


```yaml
linters:
InstanceVariables:
enabled: true
include:
- app/views/**/_*.html.slim
```

**Bad for the above configuration**

In `app/views/somewhere/_partial.html.slim`:

```slim
= @hello
```
**Good for the above configuration**

In `app/views/somewhere/show.html.slim`:

```slim
= render 'partial', hello: @hello
```
In `app/views/somewhere/_partial.html.slim`:

```slim
= hello
```

The linter allows ensuring only local variables and/or helper methods are used
in the configured set of Slim templates. This is often encouraged in Rails
partial templates.

## LineLength

Option | Description
Expand Down
34 changes: 34 additions & 0 deletions lib/slim_lint/linter/instance_variables.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# frozen_string_literal: true

module SlimLint
# Searches for instance variables in partial or other templates.
class Linter::InstanceVariables < Linter
include LinterRegistry

on_start do |_sexp|
processed_sexp = SlimLint::RubyExtractEngine.new.call(document.source)

extractor = SlimLint::RubyExtractor.new
extracted_source = extractor.extract(processed_sexp)
next if extracted_source.source.empty?

parsed_ruby = parse_ruby(extracted_source.source)
next unless parsed_ruby

report_instance_variables(parsed_ruby, extracted_source.source_map)
end

private

def report_instance_variables(parsed_ruby, source_map)
parsed_ruby.each_node do |node|
next unless node.ivar_type?

dummy_node = Struct.new(:line)
report_lint(dummy_node.new(source_map[node.loc.line]),
'Avoid instance variables in the configured ' \
"view templates (found `#{node.source}`)")
end
end
end
end
59 changes: 59 additions & 0 deletions spec/slim_lint/linter/instance_variables_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# frozen_string_literal: true

require 'spec_helper'

describe SlimLint::Linter::InstanceVariables do
include_context 'linter'

context 'with no instance variables' do
let(:slim) { <<-SLIM }
h1 Hello world!
SLIM

it { should_not report_lint }
end

context 'with instance variables in static contexts only' do
let(:slim) { <<-SLIM }
- if "@harmless_variable"
= "@another_harmless_variable"
p @and_another
' \#{"@and_another"}
SLIM

it { should_not report_lint }
end

context 'with instance variables in control code' do
let(:slim) { <<-SLIM }
- if @world
' hello world
- call(arg: @hello)
SLIM

it { should report_lint line: 1 }
it { should report_lint line: 3 }
end

context 'with instance variables in output code' do
let(:slim) { <<-SLIM }
p
' hello
= @world
== call(arg: @hello)
span = @some_variable
SLIM

it { should report_lint line: 3 }
it { should report_lint line: 4 }
it { should report_lint line: 5 }
end

context 'with instance variables in interpolated code' do
let(:slim) { <<-SLIM }
' \#{@hello}
SLIM

it { should report_lint line: 1 }
end
end

0 comments on commit 77affdc

Please sign in to comment.