New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic erb file is generating syntax errors that I don't expect, but the site works. #580

Closed
justin-capalbo opened this Issue May 25, 2017 · 17 comments

Comments

Projects
None yet
5 participants
@justin-capalbo
Copy link

justin-capalbo commented May 25, 2017

Perhaps someone can point me in the right direction here.

I have this line in my vimrc: let g:ale_echo_msg_format = '%linter% says %s' which I tried to make sure I did to confirm that it's actually the correct linter throwing the errors. The strangest part about the errors as the title says is that the site works. I can go to the page with no problems and I don't get any syntax errors thrown by Rails.

https://github.com/justin-capalbo/grocerly/blob/site-layout/app/views/devise/sessions/new.html.erb

5 - erubylint says syntax error, unexpected ')'
26 - erubylint says syntax error, unexpected keyword_end, expecting ')'
27 - erubylint says syntax error, unexpected end-of-input, expecting ')'

As I said, the site does work as expected and I don't think there's actually a problem with this .erb file. If I need to provide anything else, please let me know. The only command for ale I have in vimrc is that string display which I pasted above - other than that I just installed it using Pathogen with git clone. I couldn't find any similar issues related to ERB.

@w0rp w0rp added the bug label May 25, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 25, 2017

Maybe someone who is familiar with Ruby can have a look at this. The erb linter operates by creating Ruby code with some erb tool and then passes that on to ruby, I think.

@elebow

This comment has been minimized.

Copy link
Contributor

elebow commented May 26, 2017

The ERB linter uses erb, which is a tool that comes with the standard ruby library. I ran your code through erb from the command line and got the same errors, so ALE is behaving correctly.

Your code is valid, though; even examples from the Rails documentation will generate these errors (see https://stackoverflow.com/questions/17374274/why-is-this-an-error-with-erb and http://timelessrepo.com/block-helpers-in-rails3). The conclusion is that erb is not a reliable linter for Rails code. I also tried erubis, an alternative implementation of the ERB language, and got the same results.

I'm not sure what the path forward for ALE should be, but my first thought is to disable this linter by default on the theory that no checking at all is better than giving false positives. These patterns are common in Rails code, and most(?) uses of ERB are in Rails applications. At the very least, a note should be added to the documentation. I can write that if it's the preferred solution.

@justin-capalbo

This comment has been minimized.

Copy link

justin-capalbo commented May 26, 2017

Thanks for your research, unfortunate that it's on erb's end. I've got a snippet in vimrc disabling it already, but I'm curious what they would their developers would have to say about it. I use c9ide for most of my development right now in my fledgling stage, and editing the same ERB files in their text editor, and it actually just occurred to me that there are no visible errors even if I create code that's most certainly in error. So maybe they took the same approach with their toolset.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 26, 2017

If it doesn't work properly, I'd just remove it entirely. If you want it, you can add it to your .vim directory.

@justin-capalbo

This comment has been minimized.

Copy link

justin-capalbo commented May 26, 2017

Thanks. I'm not entirely clear on how Ale works so I'll have to thumb through and figure that out. Not sure what we do with this issue though if it's not really an issue with Ale. Thanks again!

justin-capalbo added a commit to justin-capalbo/vimrc that referenced this issue May 26, 2017

Disable .erb linter and add Neocomplete and CtrlP
The .erb linter isn't working properly and giving false positives because of a problem with Ruby itself rather than with Ale.  Reported it to the Ale dev here: w0rp/ale#580.

Added Neocomplete but haven't had any time to play with the actual features and been spending more time trying to get used to CtrlP.  Some rebinding will be needed to make it easier to use.
@elebow

This comment has been minimized.

Copy link
Contributor

elebow commented May 27, 2017

I wouldn't say there's a problem on erb's end, because the input is not valid ERB code. Rails has a mechanism that rewrites the generated code to make it valid. So none of the tools are incorrect; it's just a matter of expectations.

The ideal solution is probably a new linter that understands Rails-flavored ERB. The second best solution might be to only run the erb linter when the user is not editing a Rails app (which is something that ALE can detect 😄).

@justin-capalbo

This comment has been minimized.

Copy link

justin-capalbo commented May 27, 2017

Thanks for the clarification. Still VERY new to Rails and to Ruby in general. Very good information here and thank you both.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Jun 8, 2017

If anyone knows how to detect the different Rails ERB files, let me know.

@elebow

This comment has been minimized.

Copy link
Contributor

elebow commented Jun 15, 2017

I don't believe there exists a non-hacky, reliable way to detect the flavor of ERB in use. Rails uses regular expressions[1] to rewrite Rails-flavored ERB (which I'll call "RfERB") into standard ERB, which is then fed to a standard parser.

Incorporating those same regexes would allow ALE to lint RfERB, but would cause false negatives when a file not part of a Rails application contains RfERB syntax. That is probably a common case, since many developers learn ERB while working on Rails projects and may assume that the RfERB syntax is the only syntax. Not to mention the maintenance burden of keeping the regexes in sync with those in Rails. Therefore, I don't think this is a good approach.

An acceptable heuristic might be to assume RfERB whenever the file is inside a Rails source tree, using similar logic as in

function! s:FindRailsRoot(buffer) abort
[2]. However, I have seen rare situations where a standard ERB parser is invoked inside a Rails application, which would cause false negatives if RfERB syntax is used. This is (I think) much less likely than the false negatives in the previous paragraph.

In case neither of these approaches is acceptable, then I would recommend disabling the .erb linter by default in ALE. The most common use of this language family is in Rails projects, and the RfERB-specific syntax is a frequently-used feature in such files. Currently, ALE is not useful (or worse, misleading) when editing .erb files that contain RfERB.

[1] https://github.com/rails/rails/blob/7da8d76206271bdf200ea201f7e5a49afb9bc9e7/actionview/lib/action_view/template/handlers/erb/erubis.rb#L40

[2] This check could be augmented by also looking for /views/ in the file path, since Rails convention dictates that all .erb files be kept under a directory of that name.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Jun 15, 2017

I propose the following.

  1. Make the executable so you can use manually erubis instead. g:ale_pattern_options lets you set different options for different filename patterns.
  2. Detect the directory the file is in, and switch to erubis by default if the executable has not been configured otherwise.
@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Jun 15, 2017

Another option would be to search the file for some difference in syntax we could detect, and determine if the syntax looks like erubis or not.

@sublimecoder

This comment has been minimized.

Copy link

sublimecoder commented Aug 11, 2017

I'm experiencing this issue as well. Anyone have a fix for it or a workaround till it gets fixed?

@elebow

This comment has been minimized.

Copy link
Contributor

elebow commented Aug 12, 2017

I may have been unclear earlier. Both tools erb and erubis only understand the standard ERB language. There does not currently exist a parser for Rails-flavored ERB other than Rails itself, as far as I know.

The best we can do currently is run no linter for RfERB files, to avoid the false errors.

@sublimecoder

This comment has been minimized.

Copy link

sublimecoder commented Aug 12, 2017

That's better than opening a file to see 10 syntax errors that aren't actual errors.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Aug 12, 2017

If anyone knows a way to reliably detect Ruby-flavoured files and automatically disable the linter, go for it. Otherwise, the linter will have to be disabled manually.

@rgo

This comment has been minimized.

Copy link

rgo commented Oct 18, 2017

Other option that I'm trying (based on this issue (vim-syntastic/syntastic#708) is to replace <%= by <% before to parse it.

I changed the command in erubylint.vim:

diff --git a/ale_linters/eruby/erubylint.vim b/ale_linters/eruby/erubylint.vim
index 2ff03c3..ae5beaa 100644
--- a/ale_linters/eruby/erubylint.vim
+++ b/ale_linters/eruby/erubylint.vim
@@ -4,8 +4,8 @@
 call ale#linter#Define('eruby', {
 \   'name': 'erubylint',
 \   'executable': 'erb',
-\    'output_stream': 'stderr',
-\   'command': 'erb -P -x %t | ruby -c',
+\   'output_stream': 'stderr',
+\   'command': "ruby -rerb -e \"puts ERB.new(File.read(%t, encoding: 'BINARY').gsub('<%=','<%'), nil, '-').src\" | ruby -c",
 \   'callback': 'ale#handlers#ruby#HandleSyntaxErrors',
 \})

And force to use erubylint as unique linter: let g:ale_linters = {'eruby': ['erubylint']}

Updated: Fixed command quotes in gsub and added a full diff.

geoffharcourt added a commit to thoughtbot/dotfiles that referenced this issue Oct 27, 2017

Add ALE fix for Rails ERB templates
See this discussion on the related ALE issue:
w0rp/ale#580 (comment)

This change sets up ALE to use a custom linter for Rails ERB templates to
accommodate the syntax. Without the change, the `erb` linter raises
errors on templates within a Rails application (including blank files).

We define the `g:ale_linters` dictionary conditionally in case the user
has already defined it elsewhere. Users with existing ALE linter
customizations may need to change their definition so that they don't
wipe this setting out.

Close #556.

elebow pushed a commit to elebow/ale that referenced this issue Nov 12, 2017

Eddie Lebow
[eruby] Add GetCommand to erb linter
GetCommand conditionally adds a filter (implemented as inline Ruby code
in the command invoked) to transform some of the problematic
Rails-specific eRuby syntax. Specifically, <%= tags are replaced with
<%.

This does not reduce the effectiveness of the linter, because the
transformed code is still evaluated.

This solution was suggested by @rgo at
w0rp#580 (comment).

elebow pushed a commit to elebow/ale that referenced this issue Nov 12, 2017

Eddie Lebow
[eruby] Add GetCommand to erubis linter
GetCommand conditionally adds a filter (implemented as inline Ruby code
in the command line) to transform some of the problematic
Rails-specific eRuby syntax. Specifically, <%= tags are replaced with
<%.

This does not reduce the effectiveness of the linter, because the
transformed code is still evaluated.

This solution was suggested by @rgo at
w0rp#580 (comment).

elebow pushed a commit to elebow/ale that referenced this issue Nov 12, 2017

Eddie Lebow
[eruby] Add GetCommand to erb linter
GetCommand conditionally adds a filter (implemented as inline Ruby code
in the command line) to transform some of the problematic
Rails-specific eRuby syntax. Specifically, <%= tags are replaced with
<%.

This does not reduce the effectiveness of the linter, because the
transformed code is still evaluated.

This solution was suggested by @rgo at
w0rp#580 (comment).

elebow pushed a commit to elebow/ale that referenced this issue Nov 12, 2017

Eddie Lebow
[eruby] Add GetCommand to erubis linter
GetCommand conditionally adds a filter (implemented as inline Ruby code
in the command line) to transform some of the problematic
Rails-specific eRuby syntax. Specifically, <%= tags are replaced with
<%.

This does not reduce the effectiveness of the linter, because the
transformed code is still evaluated.

This solution was suggested by @rgo at
w0rp#580 (comment).

elebow pushed a commit to elebow/ale that referenced this issue Nov 12, 2017

Eddie Lebow
[eruby] Add GetCommand to erb linter
GetCommand conditionally adds a filter (implemented as inline Ruby code
in the command line) to transform some of the problematic
Rails-specific eRuby syntax. Specifically, <%= tags are replaced with
<%.

This does not reduce the effectiveness of the linter, because the
transformed code is still evaluated.

This solution was suggested by @rgo at
w0rp#580 (comment).

elebow pushed a commit to elebow/ale that referenced this issue Nov 12, 2017

Eddie Lebow
[eruby] Add GetCommand to erubis linter
GetCommand conditionally adds a filter (implemented as inline Ruby code
in the command line) to transform some of the problematic
Rails-specific eRuby syntax. Specifically, <%= tags are replaced with
<%.

This does not reduce the effectiveness of the linter, because the
transformed code is still evaluated.

This solution was suggested by @rgo at
w0rp#580 (comment).

elebow added a commit to elebow/ale that referenced this issue Nov 13, 2017

[eruby] Add GetCommand to erb linter
GetCommand conditionally adds a filter (implemented as inline Ruby code
in the command line) to transform some of the problematic
Rails-specific eRuby syntax. Specifically, <%= tags are replaced with
<%.

This does not reduce the effectiveness of the linter, because the
transformed code is still evaluated.

This solution was suggested by @rgo at
w0rp#580 (comment).

elebow added a commit to elebow/ale that referenced this issue Nov 13, 2017

[eruby] Add GetCommand to erubis linter
GetCommand conditionally adds a filter (implemented as inline Ruby code
in the command line) to transform some of the problematic
Rails-specific eRuby syntax. Specifically, <%= tags are replaced with
<%.

This does not reduce the effectiveness of the linter, because the
transformed code is still evaluated.

This solution was suggested by @rgo at
w0rp#580 (comment).

@w0rp w0rp closed this in f90a2d5 Nov 16, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Nov 16, 2017

Thanks to @elebow, ALE will now check if you're in a Rails project, and strip the Rails-specific syntax from the files.

jsteiner added a commit to jsteiner/dotfiles that referenced this issue Dec 20, 2017

Remove custom ERB linter
ALE now checks if you're in a Rails project and strips the rails
specific syntax.

w0rp/ale#580
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment