Skip to content
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

Add a handlebars linter to check for undefined variables in hooks #1779

Open
moretea opened this issue Feb 16, 2017 · 10 comments
Open

Add a handlebars linter to check for undefined variables in hooks #1779

moretea opened this issue Feb 16, 2017 · 10 comments
Labels
Focus:Supervisor Related to the Habitat Supervisor (core/hab-sup) component Focus :Templating Improvements or bugs related to templated content in the Supervisor Stale Type: Feature Issues that describe a new desired feature

Comments

@moretea
Copy link

moretea commented Feb 16, 2017

How to reproduce

  • create a package (or pick an existing one)
  • create or modify hooks/run to contain:
     {{nonense_var_name}}
    

Expected output

error message

Actual output

Silent error; this variable is rendered as an empty string.

@reset
Copy link
Collaborator

reset commented Feb 19, 2017

This behaviour is coming from our handlebars renderer. Undefined variables don't result in rendering errors in handlebars, so they're happily rendered in our configuration and hook files as well. We could fork the handlebars renderer or add our own, but the behaviour of the if block would change. undefined and null are two of the falsey representations to handlebars.

I think I'd rather have the application fail instead of the renderer. @habitat-sh/habitat-core-maintainers what do you guys think?

@smith
Copy link
Contributor

smith commented Feb 21, 2017

I'm in agreement with @reset for another reason: TOML doesn't have nil, so if you want something to not be set you either would have to use an empty string, which is not falsy, or false, which would probably require too much explanation ("use a string if present, or false without quotes if not present".)

I would prefer if it worked like the original suggestion, but it seems we would be fighting against the underlying technologies for a small benefit.

@smurawski
Copy link
Contributor

As as noted, we're using a handlebars rendering library and having it check for empty references would be a major change to the library.

We could probably address this in a linting tool that could inspect the template, the required pkg_binds, and the default toml and look for any undefined references in the config templates.

@smurawski smurawski added this to the Help Wanted milestone Feb 28, 2017
@smurawski smurawski changed the title Undefined variables in hooks should result in errors Add a handlebars linter to check for undefined variables in hooks Feb 28, 2017
@bixu
Copy link
Contributor

bixu commented Jul 20, 2017

Recently had a conversation with @adamhjk about linting templates. Having a linter would have the nice side effect of catching errors around changing syntax at build time instead of at runtime (which in a recent case at $job would have saved me a few days of frustration).

@qubitrenegade
Copy link
Contributor

I vote the name of the linter be roomba so I can hab studio roomba

@christophermaier
Copy link
Contributor

There is a strict mode in the handlebars library we're using. However, it was added in version 0.32.0 and we are currently stuck on version 0.28.3 for fun reasons

However, @mwrock is currently doing work to extract the core templating logic from the Supervisor into the core crate. I think that work will begin to allow us to move forward with updating our handlebars dependency, and will also make it easier to provide an external linter.

@christophermaier christophermaier added the Focus :Templating Improvements or bugs related to templated content in the Supervisor label Nov 30, 2018
@stale
Copy link

stale bot commented Apr 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

@stale stale bot added the Stale label Apr 3, 2020
@qubitrenegade
Copy link
Contributor

qubitrenegade commented Apr 3, 2020 via email

@stale stale bot removed the Stale label Apr 3, 2020
@christophermaier christophermaier added Focus:Supervisor Related to the Habitat Supervisor (core/hab-sup) component Type: Feature Issues that describe a new desired feature and removed A-supervisor labels Jul 24, 2020
@stale
Copy link

stale bot commented Sep 20, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

@stale stale bot added the Stale label Sep 20, 2022
@stale
Copy link

stale bot commented May 22, 2023

This issue has been automatically closed after being stale for 400 days. We still value your input and contribution. Please re-open the issue if desired and leave a comment with details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Focus:Supervisor Related to the Habitat Supervisor (core/hab-sup) component Focus :Templating Improvements or bugs related to templated content in the Supervisor Stale Type: Feature Issues that describe a new desired feature
Projects
None yet
Development

No branches or pull requests

9 participants