Skip to content

Conversation

@seijikun
Copy link
Contributor

@seijikun seijikun commented Jul 27, 2025

This adds an initial implementation of the filter_fn proc-macro idea pitched here.

Reading commit-by-commit will look less noisy, but the actual proc-macro implementation is one large commit either way.
The implementation has quite some logic to calculate generic parameters. I documented what is happening in the code comments as best as I could.

This:

  • Transforms existing freestanding custom filter functions (just annotate the method with #[askama::filter_fn]
  • Supports required and optional filter arguments (with default values)
  • Supports generic parameters for required filter arguments
  • Supports generic parameters for the filter's input argument

Some limitations apply. This currently:

  • Doesn't support parsing where clauses. (Bounds currently have to reside within the angle brackets)
  • Doesn't support using generic parameters for optional filter arguments
  • Doesn't support impl <Trait> notation for filter arguments
  • Doesn't support custom lifetime parameters

This is my first proc-macro. So please bear with me 😅
The current version probably doesn't provide the best compiler diagnostics possible. I plan on incrementally improving this, if you have some pointers for me.

@seijikun seijikun force-pushed the mr-filterfn branch 5 times, most recently from 083ba75 to b2be06e Compare July 27, 2025 01:11
@GuillaumeGomez
Copy link
Collaborator

If you don't mind, let's wait until https://github.com/GuillaumeGomez/askama/tree/tokenstream is finished because otherwise I'll take forever to finish my rebases. ^^'

@seijikun
Copy link
Contributor Author

seijikun commented Jul 28, 2025

If you don't mind, let's wait until https://github.com/GuillaumeGomez/askama/tree/tokenstream is finished because otherwise I'll take forever to finish my rebases. ^^'

sure, no problem!

@GuillaumeGomez
Copy link
Collaborator

I finally opened #558. Once merged and this PR has been rebased (sorry in advance for merge conflicts ><), time for reviews. :)

@seijikun seijikun force-pushed the mr-filterfn branch 2 times, most recently from cea2759 to dd0e7f9 Compare August 4, 2025 19:46
@GuillaumeGomez
Copy link
Collaborator

#558 was merged so when the merge conflicts are merged, let's review it! :)

@seijikun
Copy link
Contributor Author

seijikun commented Aug 6, 2025

The Port askama to new filter_fn proc-macro-attr commit looks probably super dumb.
I'm not quite sure whether I used everything correctly 😅
But it still works, so....

@GuillaumeGomez
Copy link
Collaborator

Looks good to me overall. Please add ui errors test to ensure we emit errors correctly if the attributes is misused or malformed and then it looks good to me.

@seijikun
Copy link
Contributor Author

seijikun commented Aug 7, 2025

Looks good to me overall. Please add ui errors test to ensure we emit errors correctly if the attributes is misused or malformed and then it looks good to me.

I just tried writing some ui tests to check some of the signature validation. But I somehow always get this as compiler output:

error[E0433]: failed to resolve: could not find `compile_error` in `core`

Did I accidentally use a nightly feature in my compiler error generation?

@GuillaumeGomez
Copy link
Collaborator

That seems surprising. What does the generated code looks like?

@seijikun
Copy link
Contributor Author

seijikun commented Aug 7, 2025

I created a new project, added askama as dependency and added this above the main function:

#[askama::filter_fn]
pub fn filter1(input: usize) {
}

This should trigger the assert: p_assert!(sig.inputs.len() >= 2, sig.span() => "Filter function missing required input and environment args")?;.
Which compiles to:

if sig.inputs.len() >= 2 {
    return Err(syn::Error::new(sig.span(), "Filter function missing required input and environment args"));
}

The generated syn::Error is propagated up into the proc-macro's entry point, where it's converted to a TokenStream and returned:

Err(err) => err.into_compile_error().into(),

I get a proper error message, with both nightly and stable compiler:
image

But inside the ui-tests, for the same code, I just get error[E0433]: failed to resolve: could not find compile_error in core.

@GuillaumeGomez
Copy link
Collaborator

Gonna try to understand what's going on when I have some time if you or @Kijewski didn't figure out by then.

@Kijewski
Copy link
Member

Kijewski commented Aug 18, 2025

I rebased the PR and moved the code generation into askama_derive. Because the function needs syn with feature "full", I un-feature-gated the block code, because it was only feature-gated so we don't have to depend on syn/full. Needing syn/full does not make me happy. :-/

Did not review the rest of the code, yet.

@seijikun
Copy link
Contributor Author

seijikun commented Aug 19, 2025

Needing syn/full does not make me happy. :-/

Yeah, it's unfortunate, but I couldn't find a way around it.

On the other hand, syn is a proc-macro dependency, so it should only ever be built once in an enduser application's lifetime and it won't be part of the resulting binary.
And if you're using askama for something bigger, you won't live a happy life without:

[profile.dev.package.askama_derive]
opt-level = 3

anyway. So I expect the effect on the enduser to be on the smaller side.

@GuillaumeGomez
Copy link
Collaborator

Do you mind fixing the merge conflicts? I'll merge once done. I can do it if you don't know how.

@seijikun seijikun force-pushed the mr-filterfn branch 2 times, most recently from ceb85d2 to 634ec4c Compare October 29, 2025 14:45
@seijikun
Copy link
Contributor Author

image

seijikun fixing the merge conflicts - 2025, colorized

@GuillaumeGomez
Copy link
Collaborator

Looks good to me, thanks! Just realized: doc is missing. Please add onto the book too. :)

@seijikun
Copy link
Contributor Author

Looks good to me, thanks! Just realized: doc is missing. Please add onto the book too. :)

This is one of the next steps noted down in: #561
I will start new PRs for these, if that's okay for you.

@GuillaumeGomez
Copy link
Collaborator

I'd prefer if docs came along the new feature directly. Simpler to keep track. :-/

@seijikun
Copy link
Contributor Author

@GuillaumeGomez Just updated the Book on custom filter implementations.
Is there any other documentation work you were missing?

Copy link
Collaborator

@GuillaumeGomez GuillaumeGomez left a comment

Choose a reason for hiding this comment

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

Just one question then it's ready. :)

@GuillaumeGomez
Copy link
Collaborator

It's ready. Sorry for taking so long and thanks a lot for your work!

@GuillaumeGomez GuillaumeGomez merged commit 650efd9 into askama-rs:main Oct 29, 2025
51 checks passed
@seijikun
Copy link
Contributor Author

seijikun commented Oct 29, 2025

It's ready. Sorry for taking so long and thanks a lot for your work!

No worries!
Thank you for all of your work on this awesome library!
We (re-)built our entire company website (https://www.ip-projects.de) based on it and are fascinated with the speed!

dog-speed

@GuillaumeGomez
Copy link
Collaborator

Wow, that's super cool. Maybe we should add a list of projects/websites using askama. :3

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.

3 participants