fix(ast-engine): panic if from_path is not implemented by bashandbone · Pull Request #98 · knitli/thread

bashandbone · 2026-03-09T21:44:53Z

Changed the default implementation of Language::from_path in crates/ast-engine/src/language.rs to use unimplemented!().
This ensures any type implementing Language that attempts to use from_path without providing a concrete implementation will panic.
Verified compilation and tested the thread-ast-engine library crates, avoiding issues with None returning silently.

PR created automatically by Jules for task 12198073203083672702 started by @bashandbone

Summary by Sourcery

Bug Fixes:

Prevent silent failures by making the default Language::from_path implementation panic when it is not overridden.

…` trait This modifies the default trait implementation of `from_path` in `crates/ast-engine/src/language.rs` to panic with `unimplemented!("from_path is not implemented")` rather than silently returning `None`. This enforces proper implementation by types implementing the `Language` trait and prevents silent failures. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

google-labs-jules · 2026-03-09T21:44:54Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

sourcery-ai · 2026-03-09T21:44:58Z

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates the default implementation of the Language::from_path method so that any Language implementor that relies on the default and calls from_path will now panic via unimplemented!() instead of silently returning None.

Class diagram for updated Language trait default from_path implementation

classDiagram
    class Language {
        <<trait>>
        +clone() Language
        +kind_to_id(kind str) u16
        +display_name() str
        +supports_file(path Path) bool
        +from_path(path Path) Option~Language~
    }

    note for Language "from_path now has a default implementation that panics via unimplemented instead of returning None when not overridden by implementors"

File-Level Changes

Change	Details	Files
Change default Language::from_path behavior from silent None to explicit panic via unimplemented!().	Replace the default return value None in Language::from_path with unimplemented!("from_path is not implemented") so that missing concrete implementations fail loudly at runtime Retain the function signature returning Option while changing only the default implementation behavior	`crates/ast-engine/src/language.rs`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey - I've left some high level feedback:

Instead of using unimplemented!() as the default, consider removing the default body from from_path so that implementors are forced at compile time to provide an implementation rather than encountering a runtime panic.
If you keep the panic-based default, it would be more actionable to include the type name (e.g., via std::any::type_name::<Self>()) in the message so it's clear which Language implementation is missing from_path.
For types that intentionally do not support from_path, it may be clearer to have them explicitly override from_path (e.g., to return None or a tailored panic) instead of relying on a generic unimplemented!() default.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Instead of using `unimplemented!()` as the default, consider removing the default body from `from_path` so that implementors are forced at compile time to provide an implementation rather than encountering a runtime panic.
- If you keep the panic-based default, it would be more actionable to include the type name (e.g., via `std::any::type_name::<Self>()`) in the message so it's clear which `Language` implementation is missing `from_path`.
- For types that intentionally do not support `from_path`, it may be clearer to have them explicitly override `from_path` (e.g., to return `None` or a tailored panic) instead of relying on a generic `unimplemented!()` default.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

Copilot

Pull request overview

This PR tightens the thread_ast_engine::language::Language trait contract by making the default Language::from_path implementation panic, preventing silent None returns when an implementor forgets to provide a concrete implementation.

Changes:

Replaced the default Language::from_path body from None to unimplemented!("from_path is not implemented").

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-09T21:47:27Z

crates/ast-engine/src/language.rs

@@ -69,8 +69,7 @@ pub trait Language: Clone + std::fmt::Debug + Send + Sync + 'static {
    }
    /// Return the file language from path. Return None if the file type is not supported.


The doc comment says from_path returns None when the file type is unsupported, but the default implementation now panics via unimplemented!(). Update the documentation to reflect that the default implementation panics and that implementors should override it to return None for unsupported paths (if that’s the intended contract).

Suggested change

/// Return the file language from path. Return None if the file type is not supported.

/// Return the file language inferred from a filesystem path.

///

/// The *default* implementation is not implemented and will panic if called.

/// Implementors should override this method and return `Some(Self)` when the

/// file type is supported and `None` when it is not.

Updated the docstring to reflect that the default implementation panics via unimplemented!() and must be overridden.

Copilot · 2026-03-09T21:47:28Z

crates/ast-engine/src/language.rs

    fn from_path<P: AsRef<Path>>(_path: P) -> Option<Self> {
-        // TODO: throw panic here if not implemented properly?
-        None
+        unimplemented!("from_path is not implemented")


The panic message "from_path is not implemented" is likely to be hard to diagnose in downstream crates with multiple Language impls. Consider including the concrete Self type name and/or guidance (e.g., "override Language::from_path for this type") to make the failure actionable.

Suggested change

unimplemented!("from_path is not implemented")

unimplemented!(

"Language::from_path is not implemented for type `{}`. \

Override Language::from_path for this type if path-based detection is required.",

std::any::type_name::<Self>()

)

Excellent point! Improved the panic message to include the dynamic type using std::any::type_name::<Self>().

Copilot · 2026-03-09T21:47:28Z

crates/ast-engine/src/language.rs

    fn from_path<P: AsRef<Path>>(_path: P) -> Option<Self> {
-        // TODO: throw panic here if not implemented properly?
-        None
+        unimplemented!("from_path is not implemented")
    }


This changes Language::from_path from returning None by default to panicking, but there’s no regression test capturing the new contract. Add a unit test that asserts calling from_path on a Language impl that doesn’t override it will panic (and optionally that overriding implementations still work).

Added tests verifying the panic string matches the expected message and confirming that explicit overrides return Some(Self) safely.

The previous run failed because of a `clippy::collapsible_if` warning due to the `-D warnings` setup in GitHub Actions. Replaced nested if block with an `&&` condition in `crates/flow/src/incremental/analyzer.rs` to fix this issue. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

Fixed misspelled words in `_typos.toml` to prevent CI failures. 1. Replaced "inout" with "input" in `classifications/_universal_rules.json`. 2. Appended "inout", "ba", "ede", and "Bare" to `_typos.toml` `extend-ignore-identifiers-re` config since they were used as valid references/names in other locations. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

…failure on stable toolchains The usage of the nightly `#![feature(trait_alias)]` caused build failures on standard stable release channels used across standard checks. Replaced the `Doc` trait alias with a regular trait definition `pub trait Doc: Clone + 'static {}` and a blanket implementation to remain compliant with stable constraints. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

bashandbone · 2026-03-09T23:30:31Z

@copilot open a new pull request to apply changes based on the comments in this thread and this one

Copilot · 2026-03-09T23:30:38Z

@bashandbone I've opened a new pull request, #102, to work on those changes. Once the pull request is ready, I'll request review from you.

…ult panic behavior Added unit tests to `crates/ast-engine/src/language.rs` to confirm the panic message format correctly identifies the missing implementation type name and verify an explicit override safely succeeds. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

…ics and contract tests (#102) * Initial plan * fix(ast-engine): improve Language::from_path doc, panic message, and add tests Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> --------- Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 9, 2026 21:44

Copilot started reviewing on behalf of bashandbone March 9, 2026 21:45 View session

sourcery-ai bot reviewed Mar 9, 2026

View reviewed changes

Copilot AI reviewed Mar 9, 2026

View reviewed changes

google-labs-jules bot and others added 3 commits March 9, 2026 21:51

Copilot AI mentioned this pull request Mar 9, 2026

fix(ast-engine): harden Language::from_path default — better diagnostics and contract tests #102

Merged

google-labs-jules bot and others added 2 commits March 9, 2026 23:35

bashandbone merged commit 506ab17 into main Mar 10, 2026
16 checks passed

bashandbone deleted the fix-language-from-path-12198073203083672702 branch March 10, 2026 00:09

		@@ -69,8 +69,7 @@ pub trait Language: Clone + std::fmt::Debug + Send + Sync + 'static {
		}
		/// Return the file language from path. Return None if the file type is not supported.

-    /// Return the file language from path. Return None if the file type is not supported.
+    /// Return the file language inferred from a filesystem path.
+    ///
+    /// The *default* implementation is not implemented and will panic if called.
+    /// Implementors should override this method and return `Some(Self)` when the
+    /// file type is supported and `None` when it is not.

Conversation

bashandbone commented Mar 9, 2026 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

google-labs-jules bot commented Mar 9, 2026

Uh oh!

sourcery-ai bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

Class diagram for updated Language trait default from_path implementation

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

google-labs-jules bot Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

google-labs-jules bot Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

google-labs-jules bot Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

bashandbone commented Mar 9, 2026

Uh oh!

Copilot AI commented Mar 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bashandbone commented Mar 9, 2026 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Mar 9, 2026 •

edited

Loading