Merge branch 'main' into francisfuzz/docs-hardcodedstring

.github/workflows/cla.yml

@@ -0,0 +1,22 @@
+name: Contributor License Agreement (CLA)
+
+on:
+  pull_request_target:
+    types: [opened, synchronize]
+  issue_comment:
+    types: [created]
+
+jobs:
+  cla:
+    runs-on: ubuntu-latest
+    if: |
+      (github.event.issue.pull_request
+        && !github.event.issue.pull_request.merged_at
+        && contains(github.event.comment.body, 'signed')
+      )
+      || (github.event.pull_request && !github.event.pull_request.merged)
+    steps:
+      - uses: Shopify/shopify-cla-action@v1
+        with:
+          github-token: ${{ secrets.GITHUB_TOKEN }}
+          cla-token: ${{ secrets.CLA_TOKEN }}

.github/workflows/linting.yml

@@ -6,7 +6,7 @@ jobs:
   test:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
     - uses: ruby/setup-ruby@v1
       with:
         ruby-version: '3.0'

.github/workflows/test.yml

@@ -5,13 +5,13 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        ruby: [2.7, '3.0', 3.1, head]
+        ruby: [2.7, '3.0', 3.1, 3.2, head]
         gemfile: [rubocop-next, rubocop-old]
     runs-on: ubuntu-latest
     env: # $BUNDLE_GEMFILE must be set at the job level, so it is set for all steps
       BUNDLE_GEMFILE: gemfiles/Gemfile.${{ matrix.gemfile }}
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
     - uses: ruby/setup-ruby@v1
       with:
         ruby-version: ${{ matrix.ruby }}

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    erb_lint (0.2.0)
+    erb_lint (0.4.0)
       activesupport
       better_html (>= 2.0.1)
       parser (>= 2.7.1.4)
@@ -12,13 +12,13 @@ PATH
 GEM
   remote: https://rubygems.org/
   specs:
-    actionview (7.0.3.1)
-      activesupport (= 7.0.3.1)
+    actionview (7.0.4.3)
+      activesupport (= 7.0.4.3)
       builder (~> 3.1)
       erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.1, >= 1.2.0)
-    activesupport (7.0.3.1)
+    activesupport (7.0.4.3)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 1.6, < 2)
       minitest (>= 5.1)
@@ -32,30 +32,30 @@ GEM
       parser (>= 2.4)
       smart_properties
     builder (3.2.4)
-    concurrent-ruby (1.1.10)
+    concurrent-ruby (1.2.2)
     crass (1.0.6)
     diff-lcs (1.5.0)
-    erubi (1.11.0)
+    erubi (1.12.0)
     fakefs (1.5.1)
     i18n (1.12.0)
       concurrent-ruby (~> 1.0)
-    loofah (2.18.0)
+    loofah (2.19.1)
       crass (~> 1.0.2)
       nokogiri (>= 1.5.9)
-    mini_portile2 (2.8.0)
-    minitest (5.16.3)
-    nokogiri (1.13.8)
+    mini_portile2 (2.8.1)
+    minitest (5.18.0)
+    nokogiri (1.14.3)
       mini_portile2 (~> 2.8.0)
       racc (~> 1.4)
     parallel (1.22.1)
     parser (3.1.2.1)
       ast (~> 2.4.1)
-    racc (1.6.0)
+    racc (1.6.2)
     rails-dom-testing (2.0.3)
       activesupport (>= 4.2.0)
       nokogiri (>= 1.6)
-    rails-html-sanitizer (1.4.3)
-      loofah (~> 2.3)
+    rails-html-sanitizer (1.5.0)
+      loofah (~> 2.19, >= 2.19.1)
     rainbow (3.1.1)
     rake (13.0.6)
     regexp_parser (2.5.0)
@@ -88,7 +88,7 @@ GEM
       rubocop (~> 1.30)
     ruby-progressbar (1.11.0)
     smart_properties (1.17.0)
-    tzinfo (2.0.5)
+    tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.1.0)
 

README.md

@@ -74,6 +74,18 @@ Make sure to add `**/` to exclude patterns; it matches the target files' absolut
 ## Enable or disable default linters
 `EnableDefaultLinters`: enables or disables default linters. [Default linters](#Linters) are enabled by default.
 
+## Disable rule at offense-level
+You can disable a rule by placing a disable comment in the following format:
+
+Comment on offending lines
+```.erb
+<hr /> <%# erblint:disable SelfClosingTag %>
+```
+
+To raise an error when there is a useless disable comment, enable `NoUnusedDisable`.
+
+To disable inline comments and report all offenses, set `--disable-inline-configs` option.
+
 ## Exclude
 
 You can specify the exclude patterns both of global and lint-local.
@@ -94,6 +106,7 @@ linters:
 |-------------------------------------------------------|:-------:|-------------------------------------------------------------------------------------------------------------------------------------------|
 | [AllowedScriptType](#AllowedScriptType)               | Yes     | prevents the addition of `<script>` tags that have `type` attributes that are not in a white-list of allowed values                       |
 | ClosingErbTagIndent                                   | Yes     |                                                                                                                                           |
+| [CommentSyntax](#CommentSyntax)                       | Yes     | detects bad ERB comment syntax                                                                                                            |
 | ExtraNewline                                          | Yes     |                                                                                                                                           |
 | [FinalNewline](#FinalNewline)                         | Yes     | warns about missing newline at the end of a ERB template                                                                                  |
 | [NoJavascriptTagHelper](#NoJavascriptTagHelper)       | Yes     | prevents the usage of Rails' `javascript_tag`                                                                                             |
@@ -110,8 +123,7 @@ linters:
 | [ErbSafety](#ErbSafety)                               | No      | detects unsafe interpolation of ruby data into various javascript contexts and enforce usage of safe helpers like `.to_json`.             |
 | [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)               |
-
+| [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)            |
 
 ### DeprecatedClasses
 
@@ -489,6 +501,7 @@ Linter-Specific Option    | Description
 `allow_blank`             | True or false, depending on whether or not the `type` attribute may be omitted entirely from a `<script>` tag. Defaults to `true`.
 `disallow_inline_scripts` | Do not allow inline `<script>` tags anywhere in ERB templates. Defaults to `false`.
 
+
 ### HardCodedString
 
 `HardCodedStrings` warns if there is a visible hardcoded string in the DOM. It does not check for a hardcoded string nested inside a JavaScript tag.
@@ -528,6 +541,26 @@ class I18nCorrector
     end
   end
 end
+
+## 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.
+
+```erb
+Bad ❌
+<% # This is a Ruby comment %>
+Good ✅
+<%# This is an ERB comment %>
+
+Bad ❌
+<% # This is a Ruby comment; it can fail to parse. %>
+Good ✅
+<%# This is an ERB comment; it is parsed correctly. %>
+
+Good ✅
+<%
+  # This is a multi-line ERB comment.
+%>
 ```
 
 ## Custom Linters
@@ -633,6 +666,48 @@ app/views/users/_graph.html.erb:27:37: Extra space detected where there should b
 2 error(s) were found in ERB files
 ```
 
+### JUnit
+
+```sh
+erblint --format junit
+<?xml version="1.0" encoding="UTF-8"?>
+<testsuite name="erblint" tests="2" failures="2">
+  <properties>
+    <property name="erb_lint_version" value="%{erb_lint_version}"/>
+    <property name="ruby_engine" value="%{ruby_engine}"/>
+    <property name="ruby_version" value="%{ruby_version}"/>
+    <property name="ruby_patchlevel" value="%{ruby_patchlevel}"/>
+    <property name="ruby_platform" value="%{ruby_platform}"/>
+  </properties>
+  <testcase name="app/views/subscriptions/_loader.html.erb" file="app/views/subscriptions/_loader.html.erb" lineno="1">
+    <failure message="SpaceInHtmlTag: Extra space detected where there should be no space." type="SpaceInHtmlTag">
+      <![CDATA[SpaceInHtmlTag: Extra space detected where there should be no space. at app/views/subscriptions/_loader.html.erb:1:7]]>
+    </failure>
+  </testcase>
+  <testcase name="app/views/application/index.html.erb" file="app/views/subscriptions/_menu.html.erb"/>
+</testsuite>
+```
+
+## Caching
+
+The cache is currently opt-in - to turn it on, use the `--cache` option:
+
+```sh
+erblint --cache ./app
+Cache mode is on
+Linting 413 files with 15 linters...
+File names pruned from the cache will be logged
+
+No errors were found in ERB files
+```
+
+Cached lint results are stored in the `.erb-lint-cache` directory by default, though a custom directory can be provided
+via the `--cache-dir` option. Cache filenames are computed with a hash of information about the file and `erb-lint` settings.
+These files store instance attributes of the `CachedOffense` object, which only contain the `Offense` attributes
+necessary to restore the results of running `erb-lint` for output. The cache also automatically prunes outdated files each time it's run.
+
+You can also use the `--clear-cache` option to delete the cache file directory.
+
 ## License
 
 This project is released under the [MIT license](LICENSE.txt).

erb_lint.gemspec

@@ -7,8 +7,8 @@ require "erb_lint/version"
 Gem::Specification.new do |s|
   s.name = "erb_lint"
   s.version = ERBLint::VERSION
-  s.authors = ["Justin Chan"]
-  s.email = ["justin.the.c@gmail.com"]
+  s.authors = ["Justin Chan", "Shopify Developers"]
+  s.email = ["ruby@shopify.com"]
   s.summary = "ERB lint tool"
   s.description = "ERB Linter tool."
   s.homepage = "https://github.com/Shopify/erb-lint"

gemfiles/Gemfile.rubocop-next

@@ -2,4 +2,4 @@
 
 eval_gemfile "../Gemfile"
 
-gem "rubocop", ">= 0.87"
+gem "rubocop", ">= 1.38.0"

lib/erb_lint/all.rb

@@ -3,6 +3,8 @@
 require "rubocop"
 
 require "erb_lint"
+require "erb_lint/cache"
+require "erb_lint/cached_offense"
 require "erb_lint/corrector"
 require "erb_lint/file_loader"
 require "erb_lint/linter_config"

lib/erb_lint/cache.rb

@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+module ERBLint
+  class Cache
+    CACHE_DIRECTORY = ".erb-lint-cache"
+
+    def initialize(config, cache_dir = nil)
+      @config = config
+      @cache_dir = cache_dir || CACHE_DIRECTORY
+      @hits = []
+      @new_results = []
+      puts "Cache mode is on"
+    end
+
+    def get(filename, file_content)
+      file_checksum = checksum(filename, file_content)
+      begin
+        cache_file_contents_as_offenses = JSON.parse(
+          File.read(File.join(@cache_dir, file_checksum))
+        ).map do |offense_hash|
+          ERBLint::CachedOffense.new(offense_hash)
+        end
+      rescue Errno::ENOENT
+        return false
+      end
+      @hits.push(file_checksum)
+      cache_file_contents_as_offenses
+    end
+
+    def set(filename, file_content, offenses_as_json)
+      file_checksum = checksum(filename, file_content)
+      @new_results.push(file_checksum)
+
+      FileUtils.mkdir_p(@cache_dir)
+
+      File.open(File.join(@cache_dir, file_checksum), "wb") do |f|
+        f.write(offenses_as_json)
+      end
+    end
+
+    def close
+      prune_cache
+    end
+
+    def prune_cache
+      if hits.empty?
+        puts "Cache being created for the first time, skipping prune"
+        return
+      end
+
+      cache_files = Dir.new(@cache_dir).children
+      cache_files.each do |cache_file|
+        next if hits.include?(cache_file) || new_results.include?(cache_file)
+
+        File.delete(File.join(@cache_dir, cache_file))
+      end
+    end
+
+    def cache_dir_exists?
+      File.directory?(@cache_dir)
+    end
+
+    def clear
+      return unless cache_dir_exists?
+
+      puts "Clearing cache by deleting cache directory"
+      FileUtils.rm_r(@cache_dir)
+    end
+
+    private
+
+    attr_reader :config, :hits, :new_results
+
+    def checksum(filename, file_content)
+      digester = Digest::SHA1.new
+      mode = File.stat(filename).mode
+
+      digester.update(
+        "#{mode}#{config.to_hash}#{ERBLint::VERSION}#{file_content}"
+      )
+      digester.hexdigest
+    rescue Errno::ENOENT
+      # Spurious files that come and go should not cause a crash, at least not
+      # here.
+      "_"
+    end
+  end
+end