Fixes #37281 - Add erb linting

[dosas]

I have seen some discussions about style of erb templates. IMHO the best solution to get rid of those is to use a linter.

Currently this only works for html,js,text, and json templates see glob expression

I doubt that we can extend that to ansible, shell, ... templates

especially because of rules like:

• SpaceInHtmlTag
• SpaceAroundErbTag
• ParserErrors
• SelfClosingTag

But I think this is a good starting point

[theforeman-bot]

Can one of the admins verify this patch?

[theforeman-bot]

Can one of the admins verify this patch?

[theforeman-bot]

Can one of the admins verify this patch?

[nadjaheitmann]

@stejskalleos I think @dosas hit a nerve here. I have run into various issues already because of linting errors in erb templates that were overlooked during review. Any idea who is the right person to ping for this PR? It does add a lot of changes, indeed.

[stejskalleos]

Assigning myself, will take a look

[nadjaheitmann]

LGTM, the changes look sane and safe, I guess we could benefit from ErbSafety and Rubocop rules in the future. The gem itself seems to be maintained.

I'd let someone else express their concerns if any.

Thanks for your review. So who else would you trigger?

[ofedoren]

LGTM, the changes look sane and safe, I guess we could benefit from ErbSafety and Rubocop rules in the future. The gem itself seems to be maintained. I'd let someone else express their concerns if any.

Thanks for your review. So who else would you trigger?

I guess Leos is already assigned to this :)

[dosas]

I like the effort; erb linting is something we could profit from in the core and the plugins. This brings me to the theforeman-rubocop plugin. I think the changes should be first introduced there and then added to the core and plugins.

Pinging @archanaserver

@stejskalleos what is the connection to rubocop?

[nadjaheitmann]

I like the effort; erb linting is something we could profit from in the core and the plugins. This brings me to the theforeman-rubocop plugin. I think the changes should be first introduced there and then added to the core and plugins. Pinging @archanaserver

@stejskalleos what is the connection to rubocop?

@dosas The Ruby code inside the erb templates is linted by erb lint (also by rubocop if I see it correctly) and this needs to go in line with the rest of the ruby code.

@archanaserver Any idea how to connect this such that the linting is unified?

[dosas]

I like the effort; erb linting is something we could profit from in the core and the plugins. This brings me to the theforeman-rubocop plugin. I think the changes should be first introduced there and then added to the core and plugins. Pinging @archanaserver

@stejskalleos what is the connection to rubocop?

@dosas The Ruby code inside the erb templates is linted by erb lint (also by rubocop if I see it correctly) and this needs to go in line with the rest of the ruby code.

@nadjaheitmann no, it is not:

 Rubocop:
    enabled: false

@archanaserver Any idea how to connect this such that the linting is unified?

[nadjaheitmann]

@nadjaheitmann no, it is not:

 Rubocop:
    enabled: false

True, I misread it. Should it be enabled? Do it in two steps and lint the ruby code part in a second step? Then it would be independent of this PR?

[archanaserver]

@archanaserver Any idea how to connect this such that the linting is unified?

@stejskalleos @nadjaheitmann To integrate ERB linting with our existing theforeman-rubocop setup, we need to add a new configuration file in theforeman-rubocop specifically for ERB linting, which will include the rules you mentioned in the .erb-lint.yml file. Also we must have to ensure here that the new configuration file is compatible with the existing RuboCop setup and does not conflict with other linting rules.

@ekohl, what do you think about this approach, thoughts?

[dosas]

To integrate ERB linting with our existing theforeman-rubocop setup, we need to add a new configuration file in theforeman-rubocop specifically for ERB linting, which will include the rules you mentioned in the .erb-lint.yml file.

So you want to include the rules for erb linting in theforeman-rubocop repo even though they are not connected?

Also we must have to ensure here that the new configuration file is compatible with the existing RuboCop setup and does not conflict with other linting rules.

[archanaserver]

So you want to include the rules for erb linting in theforeman-rubocop repo even though they are not connected?

@dosas what do you mean by "not connected"? I'm just trying to understand, I might be missing something here. But this is what I was talking about-

inherit_gem:
  theforeman-rubocop:
    - .erb-lint.yml

I was referencing this- https://github.com/Shopify/erb-lint?tab=readme-ov-file#configuration

[dosas]

@dosas what do you mean by "not connected"? I'm just trying to understand, I might be missing something here. But this is what I was talking about-

inherit_gem:
  theforeman-rubocop:
    - .erb-lint.yml

I was referencing this- https://github.com/Shopify/erb-lint?tab=readme-ov-file#configuration

@archanaserver Okay, so we are talking about the same thing: Inheriting the linting rules for rubocop from foreman-rubocop. This makes sense only if we enable this setting

  Rubocop:
    enabled: false

Enabling this setting throws ~ 6000 warnings/errors so this is definetly out of scope and should be done in a follow up PR.

Although I think your snippet is not correct, shouldn't it be.

rubocop_config:
      inherit_from:
        - .rubocop.yml

This inherits from .rubocop.yml which already uses foreman-rubocop.

From @stejskalleos original comment I got that he wants to include the erb-linting rules into rubocop. Which IMO is not connected and does not make sense

I like the effort; erb linting is something we could profit from in the core and the plugins. This brings me to the theforeman-rubocop plugin. I think the changes should be first introduced there and then added to the core and plugins.

[ehelms]

From @stejskalleos original comment I got that he wants to include the erb-linting rules into rubocop. Which IMO is not connected and does not make sense

As I understand our goal, and the request, theforeman-rubocop is designed to provide a single place for rule standardization and easy inclusion of linting rules by all of our projects. And yes, while this has "rubocop" in the name and was the original focus, I think the point is to centralize our linting rules to easily apply them across our projects.

Sounds like we then could:

1. Throw this in theforeman-rubocop and not worry about the name
2. Change theforeman-rubocop to theforeman-linting and include both
3. Create another project theforeman-erb-linting with the same model

[dosas]

From @stejskalleos original comment I got that he wants to include the erb-linting rules into rubocop. Which IMO is not connected and does not make sense

As I understand our goal, and the request, theforeman-rubocop is designed to provide a single place for rule standardization and easy inclusion of linting rules by all of our projects. And yes, while this has "rubocop" in the name and was the original focus, I think the point is to centralize our linting rules to easily apply them across our projects.

Sounds like we then could:

1. Throw this in theforeman-rubocop and not worry about the name

2. Change `theforeman-rubocop` to `theforeman-linting` and include both

3. Create another project `theforeman-erb-linting` with the same model

@ehelms Thank you for the clarification!

I'm fine with either option, so I would go for the easiest (1). Do we need an RFC for that or should I just create a PR in theforeman-rubocop?

[ehelms]

I think a PR is sufficient - let's double check whether @ekohl feels this rises to the level of an RFC