feat: static variable analysis #770

jg-rp · 2024-11-16T12:45:12Z

Statically analyze templates and report variable usage.

Usage

Retrieve the names of variables used in a template with Liquid.variables(template). It returns an array of strings, one string for each distinct variable, without its properties.

import { Liquid } from 'liquidjs'

const engine = new Liquid()

const template = engine.parse(`\
<p>
  {% assign title = user.title | capitalize %}
  {{ title }} {{ user.first_name | default: user.name }} {{ user.last_name }}
  {% if user.address %}
    {{ user.address.line1 }}
  {% else %}
    {{ user.email_addresses[0] }}
    {% for email in user.email_addresses %}
       - {{ email }}
    {% endfor %}
  {% endif %}
<p>
`)

console.log(engine.variablesSync(template))

Output

[ 'user', 'title', 'email' ]

Alternatively, use Liquid.fullVariables(template) to get a list of variables including their properties. Notice that variables from tag and filter arguments are included too.

// continued from above
engine.fullVariables(template).then(console.log)

Output

[
  'user.title',
  'user.first_name',
  'user.name',
  'user.last_name',
  'user.address',
  'user.address.line1',
  'user.email_addresses[0]',
  'user.email_addresses',
  'title',
  'email'
]

Or use Liquid.variableSegments(template) to get an array of strings and numbers that make up each variable's path.

// continued from above
engine.variableSegments(template).then(console.log)

Output

[
  [ 'user', 'title' ],
  [ 'user', 'first_name' ],
  [ 'user', 'name' ],
  [ 'user', 'last_name' ],
  [ 'user', 'address' ],
  [ 'user', 'address', 'line1' ],
  [ 'user', 'email_addresses', 0 ],
  [ 'user', 'email_addresses' ],
  [ 'title' ],
  [ 'email' ]
]

Global Variables

Notice, in the examples above, that title and email are included in the results. Often you'll want to exclude names that are in scope from {% assign %} tags, and temporary variables like those introduced by a {% for %} tag.

To get names that are expected to be global, that is, provided by application developers rather than template authors, use the globalVariables, globalFullVariables or globalVariableSegments methods (or their synchronous equivalents) of a Liquid class instance.

// continued from above
engine.globalVariableSegments(template).then(console.log)

Output

[
  [ 'user', 'title' ],
  [ 'user', 'first_name' ],
  [ 'user', 'name' ],
  [ 'user', 'last_name' ],
  [ 'user', 'address' ],
  [ 'user', 'address', 'line1' ],
  [ 'user', 'email_addresses', 0 ],
  [ 'user', 'email_addresses' ]
]

Partial Templates

By default, LiquidJS will try to load and analyze any included and rendered templates too.

import { Liquid } from 'liquidjs'

const footer = `\
<footer>
  <p>&copy; {{ "now" | date: "%Y" }} {{ site_name }}</p>
  <p>{{ site_description }}</p>
</footer>`

const engine = new Liquid({ templates: { footer } })

const template = engine.parse(`\
<body>
  <h1>Hi, {{ you | default: 'World' }}!</h1>
  {% assign some = 'thing' %}
  {% include 'footer' %}
</body>
`)

engine.globalVariables(template).then(console.log)

Output

[ 'you', 'site_name', 'site_description' ]

You can disable analysis of partial templates by setting the partials options to false.

// continue from above
engine.globalVariables(template, { partials: false }).then(console.log)

Output

[ 'you' ]

If an {% include %} tag uses a dynamic template name (one that can't be determined without rendering the template) it will be ignored, even if partials is set to true.

coveralls · 2024-11-16T12:47:49Z

Pull Request Test Coverage Report for Build 11894990314

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

168 of 216 (77.78%) changed or added relevant lines in 24 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage decreased (-2.0%) to 97.97%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/tags/block.ts	0	1	0.0%
src/tags/for.ts	7	9	77.78%
src/tags/tablerow.ts	5	7	71.43%
src/tags/include.ts	1	8	12.5%
src/tags/layout.ts	1	8	12.5%
src/template/analysis.ts	100	112	89.29%
src/tags/render.ts	1	18	5.56%

Files with Coverage Reduction	New Missed Lines	%
src/drop/blank-drop.ts	1	90.91%

Totals
Change from base Build 11800996948:	-2.0%
Covered Lines:	2700
Relevant Lines:	2749

💛 - Coveralls

src/tags/capture.ts

src/tags/cycle.ts

harttle · 2024-11-16T15:15:25Z

test/integration/static_analysis/variables.spec.ts

+
+    expect(analysis).toStrictEqual({
+      variables: { 'd[a[b.c]]': [d], 'a[b.c]': [a], 'b.c': [bc] },
+      globals: { 'd[a[b.c]]': [d], 'a[b.c]': [a], 'b.c': [bc] },


What if d, a are global while b is local? Not sure if we can implement it thoroughly. If not, would you consider to reduce this feature to a list of top level variables:

globals: ['d', 'a', 'b']

I guess this list is already useful for new comers, am I correct here?

I've added a test with a local variable nested within a global variable. Still not sure what's best here.

Do you consider my proposal in another comment? I assume dynamic index is not important as users won't figure out its value in runtime (not sure, do you actually have such a use case that I'm missing out?). Thus we can simplize the situation. For references:

a[b.c].d a[0].c

We return

a[].d b.c a.0.c or a[0].c // considering `a.0.c` can be confusion when 0 is a string (which can contain dots), latter maybe better as you have a `variableSegments` for easier consumption anyway, this is only for inspection I guess

Note: static index like 0 or quoted strings can still be treated statically, like property access.

jg-rp · 2024-11-18T15:07:00Z

With the latest commit, I've changed several of the built-in tags to fix their row and column numbers reported from Token.getPosition().

I guess this is a good point to decide if having correct row and column is important enough to warrant these changes.

Notice that when parsing if, elsif and unless tags, we're now avoiding some string slicing. Instead, when new Value() is given a TagToken, we pass the entire input string and a range to Tokenizer, which I'm hoping will result in a performance boost rather than a performance penalty.

src/template/value.ts

test/integration/util/error.spec.ts

jg-rp · 2024-11-23T09:02:26Z

I've provisionally implemented some convenience analysis methods on the Liquid class. Please consider these (along with any other features) as ideas that can be removed before merging.

Other ideas that I've yet to implement:

Report names of Liquid filters and their locations in analysis results (I've seen people ask for this before).
Report names of tags and their locations in analysis results.
Add options to control partial template analysis. At the moment we throw an error if a partial template can't be loaded, and silently ignore partial templates included/rendered with a dynamic name.

jg-rp · 2024-12-04T11:47:39Z

@harttle 👋 , I've not forgotten about this or your previous comments about keeping track of aliases. I'll get back to it soon.

harttle · 2024-12-22T08:36:45Z

docs/source/tutorials/static-analysis.md

+]
+```
+
+Or use `Liquid.variableSegments(template)` to get an array of strings and numbers that make up each variable's path.


For fullVariables and variableSegments, can we also include an examle for nested variables, or we'll need to mention how nesting will be handled in return values of these 2.

For reference a[b.c].d, I think we have another option, return 2 references without nesting them. As what's inside [] is not important because it's dynamic anyway:

a[].d b.c

If you adopt this implementation, we'll need to decide how to represent [] in variableSegments return value. Otherwise these 2 will be the same:

arr[0].length arr.length

Maybe differentiate with nesting like:

['arr', ['length']] ['arr', 'length']

Using your example, a[b.c].d, variableSegments was incorrectly producing something like this:

[ [ 'a', Variable { segments: [ 'b', 'c' ], location: { row: 1, col: 6, file: undefined } }, 'd' ], [ 'b', 'c' ] ]

This was not my intention. Now we get the following.

variables

[ 'a', 'b' ]

fullVariables

[ 'a[b.c].d', 'b.c' ]`

variableSegments

[ [ 'a', [ 'b', 'c' ], 'd' ], [ 'b', 'c' ] ]

For arr[0].length and arr.length, we get [ 'arr', 0, 'length' ] and [ 'arr', 'length' ], respectively.

Is that what you had in mind?

No, I mean maybe we can drop nesting. And treat them as different references. I guess what exactly is inside ’[]’ is not important, we only know that array/map is accessed. Assuming figure out its value when called is not feasible for static analysis, then ppl won’t use that information effectively. Here’s my simplified approach

variables

[ 'a', 'b' ]
fullVariables

[
'a[].d',
'b.c'
]`
variableSegments

[
[ 'a', [ 'd' ] ],
[ 'b', 'c' ]
]

note the last one use nested array to represent entering into array, not nested index.

Ah, I see. I think your approach should be in addition to variableSegments, as separate method/s. Then users have the option of working with nested variables and array indexes, if they're useful, or your more convenient representation if they're not.

Perhaps normalizedSegments would be a good name?

It seems like we're loosing potentially valuable information if we drop nested variables altogether.

Just another idea to simplify the implementation. If you still think dynamic index info is important, we can keep your current implementation. No need to compromise for my opinion.

src/liquid.ts

jg-rp · 2024-12-22T16:47:24Z

Can you think of better names for fullVariables, globals, locals, and/or segments?

At the moment:

The name "global" or "global variables" is inspired by Python's built-in globals() function. In our case it means names added to a template's scope by application developers at render time. Global variables are available to the root template and any templates that it includes, including those rendered with the {% render %} tag.
The name "locals" or "template local variables" is inspired by Python's built-in locals() function. Here it means names that have been added to a template's scope from an {% assign %}, {% capture %}, {% increment %}, etc. tag.
"Segments" is a term used by RFC 9535. One can think of Liquid variables as paths, where each path is made up of one or more segments.
A "fullVariable" is roughly equivalent to a "normalized path" in RFC 9535.

harttle · 2024-12-22T09:09:23Z

test/integration/static_analysis/variables.spec.ts

+
+    expect(analysis).toStrictEqual({
+      variables: { 'd[a[b.c]]': [d], 'a[b.c]': [a], 'b.c': [bc] },
+      globals: { 'd[a[b.c]]': [d], 'a[b.c]': [a], 'b.c': [bc] },


Do you consider my proposal in another comment? I assume dynamic index is not important as users won't figure out its value in runtime (not sure, do you actually have such a use case that I'm missing out?). Thus we can simplize the situation. For references:

a[b.c].d a[0].c

We return

a[].d b.c a.0.c or a[0].c // considering `a.0.c` can be confusion when 0 is a string (which can contain dots), latter maybe better as you have a `variableSegments` for easier consumption anyway, this is only for inspection I guess

Note: static index like 0 or quoted strings can still be treated statically, like property access.

src/liquid.ts

harttle · 2024-12-23T01:48:02Z

docs/source/tutorials/static-analysis.md

@@ -0,0 +1,286 @@
+---


I think you also need change sidebar and en.yml, to make this file visible on sidebar.

harttle · 2024-12-23T01:58:36Z

docs/source/tutorials/static-analysis.md

+]
+```
+
+Or use `Liquid.variableSegments(template)` to get an array of strings and numbers that make up each variable's path.


No, I mean maybe we can drop nesting. And treat them as different references. I guess what exactly is inside ’[]’ is not important, we only know that array/map is accessed. Assuming figure out its value when called is not feasible for static analysis, then ppl won’t use that information effectively. Here’s my simplified approach

variables

[ 'a', 'b' ]
fullVariables

[
'a[].d',
'b.c'
]`
variableSegments

[
[ 'a', [ 'd' ] ],
[ 'b', 'c' ]
]

note the last one use nested array to represent entering into array, not nested index.

harttle · 2024-12-23T02:22:59Z

your current naming is OK for me. Sorry some comments are not submitted yesterday, causing confusion.

feat: static variable analysis

4f0c2c4

harttle reviewed Nov 16, 2024

View reviewed changes

src/tags/capture.ts Show resolved Hide resolved

harttle reviewed Nov 16, 2024

View reviewed changes

src/tags/cycle.ts Outdated Show resolved Hide resolved

harttle reviewed Nov 16, 2024

View reviewed changes

Accept any iterable from children, arguments, etc.

e7b8559

jg-rp mentioned this pull request Nov 17, 2024

feat: static analysis #767

Closed

2 tasks

Test analysis of standard tags

a69f33c

harttle reviewed Nov 18, 2024

View reviewed changes

src/template/value.ts Outdated Show resolved Hide resolved

harttle reviewed Nov 18, 2024

View reviewed changes

src/template/value.ts Outdated Show resolved Hide resolved

harttle reviewed Nov 18, 2024

View reviewed changes

test/integration/util/error.spec.ts Show resolved Hide resolved

jg-rp and others added 15 commits November 19, 2024 09:34

Merge branch 'harttle:master' into static-analysis-alternate

e5163ba

Use TagToken.tokenizer instead of creating a new one

5705bc6

Test analysis of netsted tags

2081083

Group variables by their root value

502a80d

Test analysis of nested globals and locals

5a9d192

Analyze included and rendered templates WIP

2cb9a4f

Use existing tokenizer when constructing Hash

bc6be99

Improve test coverage

7f63cef

Analyze variables from layout and block tags

0d1393b

Test analysis of Jekyll style includes

a1972ab

Handle variables that start with a nested variable

730ab19

Async analysis

c0a19e3

Test non-standard tag end to end

1a79437

Implement convenience analysis methods on the Liquid class

d9f47d6

More analysis convenience methods

67fdbe5

jg-rp added 2 commits November 23, 2024 15:28

Accept string or template array

cde3b5d

Draft static analysis docs

a3a93cc

Deduplicate variables names

2bf55db

jg-rp marked this pull request as ready for review November 24, 2024 08:12

jg-rp mentioned this pull request Dec 4, 2024

Extract Liquid.js valid and invalid variables #777

Closed

jg-rp added 5 commits December 5, 2024 12:20

Fix isolated scope global variable map

3ff787d

Coerce variables to strings instead of extending String

5c76035

Private map instead of extending Map

9770ff3

Fix e2e test

ad2333e

Tentatively implement analysis of aliased variables

f73f0d1

harttle approved these changes Dec 22, 2024

View reviewed changes

Fix nested variable segments array

e9b11f4

harttle reviewed Dec 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: static variable analysis #770

feat: static variable analysis #770

jg-rp commented Nov 16, 2024 •

edited

Loading

coveralls commented Nov 16, 2024 •

edited

Loading

harttle Nov 16, 2024

jg-rp Nov 19, 2024

harttle Dec 22, 2024

jg-rp commented Nov 18, 2024 •

edited

Loading

jg-rp commented Nov 23, 2024

jg-rp commented Dec 4, 2024 •

edited

Loading

harttle Dec 22, 2024

harttle Dec 22, 2024

harttle Dec 22, 2024

jg-rp Dec 22, 2024

harttle Dec 23, 2024

jg-rp Dec 23, 2024

harttle Dec 23, 2024

jg-rp commented Dec 22, 2024

harttle Dec 22, 2024

harttle Dec 23, 2024 •

edited

Loading

harttle Dec 23, 2024

harttle commented Dec 23, 2024

feat: static variable analysis #770

Are you sure you want to change the base?

feat: static variable analysis #770

Conversation

jg-rp commented Nov 16, 2024 • edited Loading

Global Variables

Partial Templates

coveralls commented Nov 16, 2024 • edited Loading

Pull Request Test Coverage Report for Build 11894990314

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jg-rp commented Nov 18, 2024 • edited Loading

jg-rp commented Nov 23, 2024

jg-rp commented Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jg-rp commented Dec 22, 2024

Choose a reason for hiding this comment

harttle Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harttle commented Dec 23, 2024

jg-rp commented Nov 16, 2024 •

edited

Loading

coveralls commented Nov 16, 2024 •

edited

Loading

jg-rp commented Nov 18, 2024 •

edited

Loading

jg-rp commented Dec 4, 2024 •

edited

Loading

harttle Dec 23, 2024 •

edited

Loading