Suggest filters compatible with the object type

[Poitrin]

WHY are these changes introduced?

Fixes #658

WHAT is this pull request doing?

1. Adds logic to FilterCompletionProvider to…

• determine the type of the variable/literal (string, number, array, …) sitting before the filter separator ("input type")
• suggest filters that match the specific input type, and filters whose input type is variable (i.e. for more than 1 specific type, e.g. | default)

2. Displays deprecated filters too, so that users can still see the filter they wanted to use.

How to test your changes?

For all cases (if not stated differently):

• Make sure that filters for only the current input type are suggested.
• Verify that filters for variable input types are always suggested.

array

{% assign ct = current_tags | 
{% assign c = product.collections | 
{{ current_tags | 
{{ product.collections | 

{% assign ct = current_tags %}
{{ ct | 

{{ blog.metafields | 

string

{% assign t = product.title | 
{{ page_description | 

{% assign t = product.title %}
{{ t | 

{{ 'test%40test.com' | 
{{ '' | 

{% for tag in collection.all_tags %}
  {%- if current_tags contains tag -%}
    {{ tag | 

{{ product.selected_variant.url | 

{%- assign collection_product = collection.products.first -%}
{{ collection_product.url | 

{% assign secret_potion = 'Polyjuice' | 

{%- assign text = '  Some potions create whitespace.      ' -%}
{{ text | 

number

{{ cart.total_price | 

{% assign tp = cart.total_price %}
{{ tp | 

{{ -4.2 | 

form

{{ form | 

font

{{ font.variants.first | 

media

{{ product.featured_media | 

{% for media in product.media %}
  {% if media.media_type == 'external_video' %}
    {% if media.host == 'youtube' %}
      {{ media | 

metafield

{{ shop.metafields | 

paginate

{{- paginate | 

address

{{ shop.address | 

Display all filters

In the following examples, return types have no corresponding filter (e.g. article.image has return type image, but there is no filter for image | ). Therefore, make sure that all filters are suggested.

{{ article.image | 
{{ product.metafields.information.seasonal | 
{{ product | 
{{ settings.type_header_font | 
{{ false | 

{%- assign display_price = false -%}
{{ display_price | 

Display deprecated filters

{{ 'foo.js' | hex_to
{% assign t = product.title | 

Verify that deprecated filters are crossed out, and that a deprecation reason is shown.

[mgmanzella]

very nice 🔥 added some non blocking comments and will approve after tophatting on thursday

[Poitrin]

I noticed that I initially .mapped over all filters to get all .name from filters, just to pass every label to the ShopifyLiquid::Documentation.filter_doc method, which .finds each filter again 🤦🏻‍♂️ 🤣
Has been fixed now, so that filter_to_completion gets the complete filter, with nice information like deprecated?

[Poitrin]

I finally decided not to group the filters, and outsourced this question to https://github.com/Shopify/theme-check/issues/675

[mgmanzella]

should i be seeing these deprecated filters for integers and other object types?

[Poitrin]

should i be seeing these deprecated filters for integers and other object types?

Unfortunately yes – I did not change the logic, the deprecated filters are just more prominent now 😢:
If I take article_img_url as an example, you will see that its input type is variable:

  "deprecated": true,
  "syntax": "variable | article_img_url",
  "name": "article_img_url"

Therefore, these filters – no matter if they are deprecated or not – are suggested for every type.

Among 9 variable filters, 4 are deprecated (which are the ones you see in your screenshots).

start = { 'variable' => [0, 0], 'string' => [0, 0], 'form' => [0, 0], 'font' => [0, 0],
'number' => [0, 0], 'array' => [0, 0], 'media' => [0, 0], 'metafield' => [0, 0], 'paginate' => [0, 0], 'address' => [0, 0], 'type' => [0, 0] }
ShopifyLiquid::SourceIndex.filters.each_with_object(start) do |filter, result|
  result[filter.input_type][filter.deprecated? ? 1 : 0] += 1
end          

=> {"variable"=>[5, 4], "string"=>[82, 2], "form"=>[2, 1], "font"=>[3, 0], "number"=>[17, 0], "array"=>[11, 0], "media"=>[4, 0], "metafield"=>[2, 0], "paginate"=>[1, 0], "address"=>[1, 0], "type"=>[2, 0]}

Possible solutions

1. (my fav) We update the docs so that the 4 aforementioned deprecated filters get the input type image instead. That way, they are only suggested when working with images like article.image.
2. We hide all deprecated filters. (Or at least deprecated variable filters.)

[mgmanzella]

Unfortunately yes – I did not change the logic, the deprecated filters are just more prominent now 😢:

goooot it thanks for the explanation! i think if you havent already lets capture this in an issue as a possible enhancement. for now tho i think it's a good idea to keep it as is, we can get feedback from developers what we release and see if it's confusing. but i suspect hiding the deprecated suggestion all together will be more frustrating

Possible solutions
(my fav) We update the docs so that the 4 aforementioned deprecated filters get the input type image instead. That way, they are only suggested when working with images like article.image.
We hide all deprecated filters. (Or at least deprecated variable filters.)

sorting deprecated variable suggestions to the bottom might also be a good compromise 😮

[Poitrin]

if you havent already lets capture this in an issue as a possible enhancement

→ #676 – Done :-)

[mgmanzella]

🚀 🚀 🚀