Change the way autoload paths are added to application.rb

[trinitytakei]

The Problem

As described here: https://discord.com/channels/1082611227827638303/1082612756676620318/1255932550837309573 brakeman fails to parse config.application.rb after the phlex installer adds the extra autoload paths. As a result, it mistakenly reports a "Cross-Site Request Forgery" issue, and thus the scan_ruby job in ci.yml fails. (brakeman and ci.yml with a scan_ruby job are Omakase Rails since 7.2.beta).

The Proposed Solution

After some debugging, I figured out the code to be inserted into config/application.rb that is

• functionally equivalent to the original
• can be parsed properly by brakeman (and thus there's no mistaken "Cross-Site Request Forgery" report)

🤔

This is not a phlex-rails issue per se (the original code generated by phlex:install is not 'less valid' or worse in any way than the proposed code). However, I believe the fix still makes sense, because it's a simple one that works right now, won't cause any trouble down the line, and will hopefully save some headache for Phlex + brakeman users.

The proper/long term fix of course is to handle this in brakeman. I consulted with a security expert in the know, and he confirmed that brakeman's current parser is indeed not great; Prism to the rescue (at some point in the future).

[joeldrapper]

I don’t think this line does anything, seems to be leftover from debugging.

[trinitytakei]

I think it does: closes the heredoc from line 9 ;)

Probably three occurrences of ADD_EXTRA_AUTOLOAD_PATHS_CODE and the extra empty line (on line 15) was a bit disorienting.

Not sure what's the deal with using the same string 3x (yay/nay/what's the best practice).

As for the extra line. I prefer the extra line - it's the difference between

    config.autoload_paths.push(
      "#{root}/app/views/components",
      "#{root}/app/views",
      "#{root}/app/views/layouts"
    )
    config.application_name = "railsnew.io" # look ma, no empty line above me

and

    config.autoload_paths.push(
      "#{root}/app/views/components",
      "#{root}/app/views",
      "#{root}/app/views/layouts"
    )
    
    config.application_name = "railsnew.io" # 👌👌👌

While I prefer the second version (I'm a sucker for little things like this), in this case I don't mind either way as application.rb is not a file anyone is frequently working with.

[joeldrapper]

I think it does: closes the heredoc from line 9 ;)

🤦 Oh of course! I thought it was a void call to the constant.

The extra line makes sense too.

[joeldrapper]

This looks great apart from the one issue on line 16.

[trinitytakei]

Just pushed rubocop fixes (I'm normally adding those with git commit -v -a --no-edit --amend - let me know if you prefer to have an explicit commit (and probably squash during merging, unless you are keen on keeping the history)).

[trinitytakei]

So NOW it is weird. Rubocop 'fixed' this:

		ADD_EXTRA_AUTOLOAD_PATHS_CODE = <<-ADD_EXTRA_AUTOLOAD_PATHS_CODE
    config.autoload_paths.push(
      "\#{root}/app/views/components",
      "\#{root}/app/views",
      "\#{root}/app/views/layouts"
    )

				ADD_EXTRA_AUTOLOAD_PATHS_CODE

No idea why the weird indentation?

[joeldrapper]

Sometimes Rubocop does weird auto-correct things with indentation. What do you think about this indentation?

Using <<~ so Ruby strips the leading whitespace. I believe this will only strip the tabs. Additionally, using RUBY as the delineator means some editors will highlight it as Ruby.

[joeldrapper]

Oh no, I’m wrong. It strips the spaces too. 😢

[joeldrapper]

Don’t try too hard to please Rubocop. If Rubocop is wrong, we’ll change the Rubocop configuration.

[trinitytakei]

Using <<~ so Ruby strips the leading whitespace.

Yeah, I toyed with that too, but between Rubocop and how I wanted it to look like, I culdn't figure it out.

Additionally, using RUBY as the delineator means some editors will highlight it as Ruby.

Sounds good. Heredoc syntax highlighting is underrated imo: https://mhenrixon.com/articles/heredoc-syntax-highlighting

[joeldrapper]

Good to merge after the heredoc update. 👍

[trinitytakei]

@joeldrapper Sorry if I'm missing something, but is anything required of me at this point (CI is 🟢)?

[joeldrapper]

All good and merged. Thank you!