slim-lint
slim-lint copied to clipboard
Linter ControlStatementSpacing raise incorrect `=`
It's weird why ControlStatementSpacing
linter keeps raising Please add a space before and after the =
even I didn't see any isse.
div layout="column" layout-gt-sm=="row" ng-show="search_meta.total_pages | valPresent"
.app-bg-color.md-primary.search-result flex=true
| Search Results ({{ search_meta.total_count }} {{ search_meta.total_count > 1 ? 'documents' : 'document' }})
.pagination.app-pagination[flex=true
layout="row"
layout-align="center center"
layout-align-gt-sm="end center"
ng-show="search_meta.total_pages > 1"]
md-button.app-color-hover.app-primary[ng-href="{{ searchURL({page: 1}) }}"
ng-disabled="search_meta.current_page == 1"
aria-label=online_t(:first_page, scope: %i(document actions search))]
md-icon.md-dark first_page
md-tooltip md-direction="bottom" = online_t(:first_page, scope: %i(document actions search))
md-button.app-color-hover.app-primary[ng-href="{{ searchURL({page: search_meta.prev_page}) }}"
ng-disabled="search_meta.prev_page == null"
aria-label=online_t(:prev_page, scope: %i(document actions search))]
md-icon.md-dark keyboard_arrow_left
md-button.app-primary[ng-repeat="page in searchNavPages(search_meta, search_meta.current_page)"
ng-href="{{ searchURL({page: page}) }}"
ng-class="{'app-bg-color': search_meta.current_page == page, \
'app-color-hover': search_meta.current_page != page}"
ng-bind="page"]
md-button.app-color-hover.app-primary[ng-href="{{ searchURL({page: search_meta.next_page}) }}"
ng-disabled="search_meta.next_page == null"
aria-label=online_t(:next_page, scope: %i(document actions search))]
md-icon.md-dark keyboard_arrow_right
md-tooltip md-direction="bottom" = online_t(:next_page, scope: %i(document actions search))
md-button.app-color-hover.app-primary[ng-href="{{ searchURL({page: search_meta.total_pages}) }}"
ng-disabled="search_meta.current_page == search_meta.total_pages"
aria-label=online_t(:last_page, scope: %i(document actions search))]
md-icon.md-dark last_page
md-tooltip md-direction="bottom" = online_t(:last_page, scope: %i(document actions search))
WARNINGS:
app/views/online/templates/_search_pagination.html.slim:26 [W] ControlStatementSpacing: Please add a space before and after the `=`
app/views/online/templates/_search_pagination.html.slim:31 [W] ControlStatementSpacing: Please add a space before and after the `=`
Dependencies:
slim_lint (0.17.0)
rake (>= 10, < 13)
rubocop (>= 0.50.0)
slim (>= 3.0, < 5.0)
sysexits (~> 1.1)
@mmzoo you added this linter in #82. Can you investigate?
Long time no see :D
In that code snippet there is one backslash \
that causes a weird behavior. Syntactically it is correct as per this (I don't think this minor bug affects it). Generally multi-line attributes seem not 100% robust, though.
Now, if I create a slim file with this content:
div class='one \
two'
div = some_method
and run the linter over it in master, I get this error:
test.html.slim:2 [W] ControlStatementSpacing: Please add a space before and after the `=`
"That's good, I can reproduce the bug", I thought. But when I try to add a test for it, the test passes without error!
context 'multi-line attributes' do
let(:slim) { "div class='one \\n two'\ndiv = some_method" }
it { should_not report_lint }
end
# Or even like this
context 'multi-line attributes' do
let(:slim) {
<<-END
div class='one \
two'
div = some_method
END
}
it { should_not report_lint }
end
I'm not sure what could cause this behavior. I have nothing in my .slim-lint.yml
.
(Incidentally, in this example, the code line with the backslash ends on a comma, which also serves as a newline in the compiler. So, simply removing the backslash is a workaround to get the linter pass, without affecting the generated HTML.)
Thanks for investigating!
Given there is a workaround, I'm inclined to leave this issue open for someone with enough interest to fix. Will happily merge a pull request fixing the issue (though I am at a loss as to why it would not be reproducible in the test environment).
I think this error is due to slim-lint escaping a line break when it's preceded by a backslash. You can test this by adding a character after the backslash and seeing the error disappear.
To replicate it in a test, perhaps we could escape the slash:
"div class='one \\\n two'\ndiv = some_method"
or used a non-escaped heredoc:
<<-'END'
div class='one \
two'
div = some_method
END
Either way, I think underlying is a real error that causes slim-lint's line count to be off by one for every line that ends in a backslash. Hopefully I have time to familiarize myself with the repository and pursue this - but I'll leave this comment just in case I don't.
I put out a PR for a code that solves this problem. I referred to @chris-hewitt 's comment for a test that reproduces the situation.
The problem seemed to be that https://github.com/sds/slim-lint/blob/main/lib/slim_lint/document.rb#L41 was creating source_line with a newline with or without a backslash. So, I put together a code to correct that.
This is my first time submitting a PR to OSS, so I apologize if I am rude. Also, I apologize if the English is difficult to understand due to machine translation.
Implemented in #164.