Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 23, 2025

Overview

This PR adds support for parsing identifiers that end with a question mark (?), which are commonly used in Shopify Liquid templates following Ruby naming conventions for predicate methods.

Background

Shopify Liquid allows property names and method-like identifiers with a trailing question mark, such as:

  • product.quantity_price_breaks_configured?
  • collection.products.empty?

These identifiers appear in official Shopify themes (like Dawn and Warehouse) and are used extensively in production templates. Currently, Fluid cannot parse these templates, forcing users to pre-process them by removing the trailing question marks.

Implementation

This PR introduces a new parser option AllowTrailingQuestionMark that follows the same pattern as the existing AllowLiquidTag option:

var parser = new FluidParser(new FluidParserOptions { AllowTrailingQuestionMark = true });
parser.TryParse("{% if collection.products.empty? %}No products{% endif %}", out var template, out var error);

When enabled, the parser:

  1. Accepts identifiers with a trailing ? during parsing at any position in a multi-segment expression
  2. Strips the ? before creating AST nodes (so empty? becomes empty)
  3. Resolves identifiers to normal C# properties/methods without the ?

This approach allows seamless integration with existing C# code while maintaining compatibility with Shopify Liquid template syntax.

Key Changes

  • FluidParserOptions: Added AllowTrailingQuestionMark boolean property (defaults to false)
  • IdentifierParser: Updated to optionally accept and strip trailing ? characters
  • FluidParser: Changed Identifier field from static to instance to support per-parser configuration
  • Tests: Added 25 comprehensive tests covering various scenarios including intermediate identifiers

Example Usage

var parser = new FluidParser(new FluidParserOptions { AllowTrailingQuestionMark = true });

// All of these patterns are supported:
parser.TryParse("{{ products.empty? }}", out var template, out var errors);
parser.TryParse("{{ a?.b.c }}", out var template, out var errors);  // Intermediate identifier
parser.TryParse("{{ a?.b?.c? }}", out var template, out var errors);  // Multiple identifiers with ?

var context = new TemplateContext();
var obj = new { empty = true };
context.Options.MemberAccessStrategy.Register(obj.GetType());
context.SetValue("products", obj);

var result = await template.RenderAsync(context);  // Returns "true"

Backward Compatibility

The feature is disabled by default, ensuring complete backward compatibility. Existing code continues to work unchanged, and the new behavior is only activated when explicitly enabled via the parser options.

Testing

  • ✅ All 1223 existing tests pass
  • ✅ 25 new tests added covering various use cases including:
    • Trailing ? on final identifiers
    • Trailing ? on intermediate identifiers (e.g., a?.b.c)
    • Multiple identifiers with ? (e.g., a?.b?.c?)
    • Integration with filters, assignments, if statements, for loops, etc.
  • ✅ CodeQL security scan: 0 alerts
  • ✅ Manual verification with real-world Shopify-like templates

Fixes #837

Original prompt

This section details on the original issue you should resolve

<issue_title>Support for identifiers ending with ?</issue_title>
<issue_description>I’d like to open a discussion about supporting identifiers that end with a question mark (?), which are used in Liquid.

Background

Shopify Liquid allows property names or method-like identifiers with a trailing question mark — for example:
[quantity_price_breaks_configured?](https://shopify.dev/docs/api/liquid/objects/product#product-quantity_price_breaks_configured?)

This syntax comes from Ruby conventions, where methods ending in ? are typically used for predicates.
An example from pure Liquid source is the empty? filter:

# In standardfilters.rb (Shopify Liquid)
def empty?(input)
  input.respond_to?(:empty?) && input.empty?
end

In templates, this is used as:

{% if collection.products.empty? %}
  No products found
{% endif %}

These identifiers appear in official and third-party Shopify themes (for example, Dawn and Warehouse).

Currently, Fluid does not support this syntax because IdentifierParser disallows the ? character at the end of identifiers.
As a result, a statement like product.quantity_price_breaks_configured? or collection.products.empty? fails to parse.

Why is this even needed?

We’re trying to render live templates directly from Shopify themes.
These templates use ?-terminated identifiers extensively. Without support for them, Fluid cannot parse or evaluate these templates directly, forcing workarounds.

Current workaround

We currently run a regex pre-processor over Shopify theme files to remove trailing question marks from identifiers before parsing them with Fluid.
This works, but it is a temporary workaround rather than a clean solution.

Core problem

Extending the parser to accept trailing question marks is straightforward, but that leads to a deeper question:

If we allow such identifiers in the parser, how should they be resolved on the C# side?

Since C# identifiers cannot contain ?, directly mapping them to object properties or members is not possible.
Even if parsing succeeds, evaluation fails when Fluid attempts to access object.quantity_price_breaks_configured? or object.empty? since ObjectValueBase.GetValueAsync() fetches fields and properties, and C# can't have them with a question mark in the name.

Possible approaches

  1. Parser + normalization:
    Allow identifiers ending with ?, but strip the question mark before resolution (e.g., resolve quantity_price_breaks_configured?quantity_price_breaks_configured).

  2. Parser + Custom property resolver:
    Allow identifiers ending with ?, but leave a resolver that handles ? to developers, e.g., normalize or map it to an alternate property. I suppose, if the parser allows, it should be possible via TemplateOptions.MemberAccessStrategy.Register()

Both ways may be optional, e.g, add an option such as FluidOptions.AllowTrailingQuestionInIdentifiers, so compatibility can be toggled.

Related discussion

A similar question was raised in another Liquid implementation:
cobalt-org/liquid-rust#225</issue_description>

<agent_instructions>Create a new parser option, like the AllowLiquidTag one, that enables support for ? at the end of identifiers but removed them from the resulting AST node.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes #837


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Add support for identifiers ending with question mark Add support for identifiers ending with ? in Liquid templates Oct 23, 2025
Copilot AI requested a review from sebastienros October 23, 2025 16:29
@sebastienros
Copy link
Owner

@copilot can you add a test that ensures intermediate identifiers in multi segment expression are supported?

Copy link
Contributor Author

Copilot AI commented Oct 23, 2025

@copilot can you add a test that ensures intermediate identifiers in multi segment expression are supported?

Added tests for intermediate identifiers with trailing ? in commit 1524842. The tests verify:

  • Single intermediate identifier: a?.b.c
  • Multiple intermediate identifiers: a?.b?.c?
  • Proper stripping of ? from all segments in the AST
  • Resolution and rendering of intermediate identifiers with ?

/// <summary>
/// Gets whether identifiers can end with a question mark (`?`), which will be stripped during parsing. Default is <c>false</c>.
/// </summary>
public bool AllowTrailingQuestion { get; set; }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot rename to AllowTrailingQuestionMark

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to AllowTrailingQuestionMark in commit 7f966c4. Updated all occurrences in FluidParserOptions.cs, FluidParser.cs, and all test files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for identifiers ending with ?

2 participants