Conversation
…ting compiler/lint diagnostics
…iagnostic management
…mpiler diagnostics and a new `LintHook` for lint-specific diagnostics.
…, replacing individual handlers for diagnostics, linting, preview, and export.
…ector initialization.
Summary of ChangesHello @hongjr03, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the linting system by introducing a Control Flow Graph (CFG) based dataflow analysis. This change aims to provide a more accurate and comprehensive mechanism for identifying implicitly discarded expressions within functions, particularly those affected by return statements. The new system also incorporates caching to optimize performance for repeated linting operations. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is an excellent refactoring of the linting logic, replacing the previous visitor-based dataflow analysis with a more robust and maintainable Control Flow Graph (CFG) based approach. The new implementation for detecting discarded values before a function return is well-structured, with clear separation of concerns between CFG construction, dataflow analysis, and diagnostics emission. The introduction of caching for the CFG analysis is also a great addition for performance. The code is clean, well-commented, and the changes significantly improve the clarity of the linting infrastructure. I have one minor suggestion to improve the accuracy of span information in the CFG. Overall, great work!
71da4ed to
06600ba
Compare
This PR introduces a reusable statement-level control-flow graph (CFG) and a generic forward/backward dataflow solver to
tinymist-analysis, and migrates the “implicitly discarded by function return” lint to this new infrastructure. The new implementation is more precise, path-sensitive, and cacheable, replacing the previous ad-hoc backward traversal.This PR is based on #2302 and should be merged after it.
I gated CFG-based lint behind lint-v2 feature (keep v1 as default) in 6673d33.
Motivation
The existing “discarded by function return” lint relied on a hand-written reverse traversal over blocks. While lightweight, it had several limitations:
if/else, loops,break/continue)This PR addresses these issues by introducing a proper CFG + dataflow foundation and reimplementing the lint on top of it.
What’s in this PR
1. New analysis infrastructure (
tinymist-analysis::flow)cfg: a minimal directed CFG with explicitentry/exit, stableNodeId, labeled edges, and reachability utilities.dataflow: a generic worklist-based solver for forward and backward dataflow problems over join-semilattices.typst: lowering from Typst AST to a statement-level CFG, with:if/while/for, joins, loop headers,return,break,continueand,or,not)exit)These components are reusable by future analyses and lints.
2. Rewritten “discarded by function return” lint
Implemented in
tinymist-lint/src/cfg.rsusing the new CFG + dataflow framework.Uses two backward dataflow analyses:
Semantic analysis (
MustReturnKind)Determines whether all paths from a node to
exitmust hit:returnreturn <value>return(none)Diagnostic coverage analysis
Ensures that warnings are emitted at most once per path, at the closest relevant statement to the
return.Warnings are issued only when:
return,Hints (
let _ = ...) are preserved for hashable, non-show/setexpressions.3. Removal of legacy implementation
LateFuncLinter,ReturnBlockInfo, and the old reverse block traversal.break/continuechecks no longer require tracking flags).4. Cross-request caching
LintCachesintinymist-lintfor expensive analyses.(TypstFileId, source_hash)intinymist-query.5. Tests and behavior changes
Adds new fixtures covering:
returnwithnone+setreturnwithnone+showreturnwithnone+ plain textUpdates snapshots:
breakpaths.return noneonly warns forshow/set, not plain text.