Feature Request: Discourage users passing global variables into snippets.

[ConduciveMammal]

So I've recently become aware that passing global variables into snippet renders isn't needed.

So while I've been doing something like {% render 'my-snippet', customer: customer %}, it wasn't necessary.

I wonder if it's possible to create a warning message stating this?

[PaulNewton]

Severity either a suggestion , or possibly as a matter of style in names & values, definitely not as an error for parameter usage of globals as values. Contextual naming of variables not being done which can lead to overwriting globals is a core problem here, the core problem is inability to reset globals to original values. IMO some keywords like customer should be unassignable restricted keywords at the platform level 🤷‍♂️

There's a meta issue this helps avoid too, from the shopify docs: "Predefined Liquid objects can be overridden by variables with the same name"

So excluding names in iteration tags for iterable objects[1]:

1. As a style - keywords for global objects should not be used as parameter names or values passed to sections nor snippets.
2. As a suggestion - keywords for Predefined objects should not be used as names for variable tags in other resources templates.
3. As an error - keywords for Predefined objects inside the context of that objects resource templates should not be assigned to variable|parameter names
4. As an error - keywords for global objects should never be used as names for variable tags ; but fine to pass as the value

Can swap "keywords for" with "names of" or other wording 👨‍🎓 📜 👨‍🏫. Could prepend or append "create a more contextual variable name" to most of those rules messages. For # 2 or # 3 could recommend usage of the instance convention current_[resource name] i.e. current_product when in product.liquid templates, not sure how smart rules can be 🤷‍♂️ . For # 4 a counter example is something like {% assign articles = articles | where:"" %} but that's just avoiding coming up with a contextual name. For # 4 it should be remark how the issue is that global variable cannot be reset to it's inital value, at least afaik I've never found a way.

For # 1 at least for objects like "customer" those might be set to error severity:

• There is only 1 customer if it exists(logged in)
• Meaning it is not assignable amongst a stores customers in liquid compared to {% assign collections = collections | where:"image" %}
• Themes should not treat or infer it is an assignable variable name
  • what i mean by infer - the "customer: customer" parameter pattern gives an impression that assigning that word in a section or snippet is okay or outright expected.
• if it is assigned garbage values things break and can be super obnoxious to debug.
• Which should be a lint check in and of itself? {% assign customer = "bob" %} == error

Valid reasons for this pattern {% render 'snippet', object: object %}

I laid out the progressive ruleset above based on the below to avoid hindering expressiveness vs killing edge cases.

1. Contextual naming is missing - this pattern can communicate intent & requirement of a snippet, that the snippet name may not be able to infer because naming is harder than ignoring a linter.
2. future refactoring and customization flexibility - so it's trivial to change the context for something like the following {% render 'confusingly-named-snippet', product: product %} with an re-assignment before it or most likely wrapping with iteration. Example:

   {% assign product = all_products[product_handle] %}
   {% render 'my-snippet', product: product %}

3. It's in the usage comment for sections or snippets or documentation owned by others 🤦‍♂️. another check rule 🤔?
4. It's a chore to create a contextual variable name for everything
5. quite often a contextual name for the variable is inside the section or snippet and that internal var is assigned the passed in parameter value. Or defaults to a global itself or something else; and that logic behavior may change if the passed in value IS the global.

For # 5 I cannot remember an exact example so here's some bad pseudo code

{% assign  filtered_collections = collection['collection_handle'] %}
{% render 'my-snippet', filtered_collections : filtered_collections %}

{% comment %} contents of my-snippet {% endcomment %}
{% assign starting_collections = filtered_collections%}
{% if filtered_collections == collections %}
 {% assign filtered_collection_titles = collections | where:"image" | map: "title" %}
{% endif %}
{% if filtered_collections != collections %}
 {% assign filtered = true %}
{% endif %}

vs

{% render 'my-snippet', filtered_collections: collections %}

A catch is that checks on globals could encourage the use of things like ccustomer or collectionss. Such as is the case in other languages that restrict the keyword "class" so people just use klass or classs, etc because lazy & smart; which is a syntax side effect to be avoided from ever reaching merchantland. But in practice keywords get used as parameter names and values for sections and snippets, and in loops because naming is hard unless a better suggestion is giving.

[1] check and and error when non-iterables are used in iteration tags 🤔