Support semantic tokens for format specifiers in logging calls #4003
Support semantic tokens for format specifiers in logging calls #4003kavix wants to merge 1 commit into
Conversation
…acebook#3908) Why: We want to highlight printf-style format specifiers inside logging strings, similar to how clangd handles this for C/C++. Since Ruff flags Python logging f-strings for lazy evaluation performance reasons, users are encouraged to use percent-style formatting, which should be properly highlighted for readability. What: We add a `formatSpecifier` semantic token to the LSP legends. We also introduce a lightweight heuristic to detect common logging functions (e.g. `logger.info`, `logging.debug`) to avoid costly symbol resolutions on the hot path. When a string literal argument is passed to these detected logging calls, we scan the literal against a strict Python format-specifier Regex. Why it works: To make the string regex matching work, we need access to the original source text. The `process_ast` pipeline was updated to propagate the module source string all the way down through the AST visitor, allowing us to slice the string accurately using the AST node ranges and extract format specifiers.
kavix
changed the title
|
Hi @kinto0, could you please review this PR ? Thanks! |
|
@kinto0 has imported this pull request. If you are a Meta employee, you can view this in D110947960. |
kinto0
left a comment
kinto0
left a comment
There was a problem hiding this comment.
hey, thanks for taking this on. left a few suggestions
| let disabled_ranges = disabled_ranges_for_module(ast.as_ref(), *handle.sys_info()); | ||
| let mut builder = SemanticTokenBuilder::new(limit_range, disabled_ranges); | ||
|
|
||
| let source = module_info.contents().as_str(); |
There was a problem hiding this comment.
I don't think the source is entirely necessary in processing the ast. can we instead use &ast that we already send?
we should be able to use StringLiteralValue
| interaction.shutdown().unwrap(); | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
please add a test for %% - I think it will fail
| SemanticTokenType::REGEXP, | ||
| SemanticTokenType::OPERATOR, | ||
| SemanticTokenType::DECORATOR, | ||
| SemanticTokenType::new("formatSpecifier"), |
There was a problem hiding this comment.
AI is telling me this will allocate on every match of our token. can we avoid this somehow?
| let Expr::Attribute(attr) = func else { | ||
| return false; | ||
| }; |
There was a problem hiding this comment.
should we instead make callers filter this?
| } | ||
| } | ||
|
|
||
| fn is_logger_candidate(func: &Expr) -> bool { |
There was a problem hiding this comment.
docstring here would be nice so we understand what it should do
| if start_offset.to_usize() > source.len() || end_offset.to_usize() > source.len() { | ||
| return; | ||
| } | ||
| let text = &source[start_offset.to_usize()..end_offset.to_usize()]; |
There was a problem hiding this comment.
I think we can just use string_lit.value and avoid source/range entirely here
| }; | ||
|
|
||
| let method = attr.attr.as_str(); | ||
| if 
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
2 participants
Summary
Fixes #3908
This PR adds a
formatSpecifiersemantic token for Python logging calls, similar to how clangd highlights format specifiers in C/C++.Since Ruff flags Python logging f-strings (for lazy evaluation performance reasons via rule G004), users are encouraged to use percent-style formatting. This change ensures that when users follow this best practice, their format specifiers (
%s,%d,%(name)s, etc.) are properly syntax-highlighted by the IDE for readability.Approach & Details
formatSpecifier: Added this custom token type to theSemanticTokensLegendsregistry.is_logger_candidate) to detect common logging functions (e.g.logger.info,logging.debug,LOGGER.warn) to avoid costly symbol resolutions on the hot path.OnceLockcached regex that strictly follows Python's printf-style format specifiers to scan string literals passed to the detected logging calls.process_astsemantic token builder pipeline to propagate thesourcetext string down through the visitor so that string literals can be sliced and matched against the regex.semantic_tokens_format_specifierintegration test tolsp_interactionwhich successfully asserts the LSP token mapping over the string"Hello %s %d".