Skip to content

Clean up public API leaks - #64

Merged
kaidokert merged 5 commits into
mainfrom
cleanapi
Jul 16, 2025
Merged

Clean up public API leaks#64
kaidokert merged 5 commits into
mainfrom
cleanapi

Conversation

@kaidokert

@kaidokert kaidokert commented Jul 16, 2025

Copy link
Copy Markdown
Owner

Summary by Sourcery

Tighten the public API by hiding internal types and removing an unused trait method

Enhancements:

  • Remove the default is_finished method from the ContentExtractor trait and its override in StreamContentBuilder
  • Qualify ContentExtractor’s return types as crate::Event and crate::ParseError to avoid leaking unprefixed internal types
  • Remove public exports of internal types EscapeTiming and ParserCore from the top-level lib

Summary by CodeRabbit

  • Refactor
    • Removed the is_finished method from content extraction features.
    • Removed certain public exports from the library interface.
  • Chores
    • Updated test workflows to consolidate JSON conformance tests.
    • Renamed feature flags related to remote test suites.
    • Expanded .gitignore to cover additional test data and generated files.

@sourcery-ai

sourcery-ai Bot commented Jul 16, 2025

Copy link
Copy Markdown

Reviewer's Guide

This PR cleans up public API leaks by switching trait signatures to crate-qualified types, removing the obsolete is_finished method, and hiding internal types from the public API.

Class diagram for ContentExtractor trait and related changes

classDiagram
    class ContentExtractor {
        +validate_and_extract_string() Result<Event, ParseError>
        +validate_and_extract_number(from_container_end: bool) Result<Event, ParseError>
        +extract_content(start_pos: usize, from_container_end: bool, finished: bool) Result<crate::Event, crate::ParseError>
    }
    %% Note: is_finished() has been removed from ContentExtractor

    class StreamContentBuilder {
        %% Implements ContentExtractor
        +validate_and_extract_number(from_container_end: bool) Result<Event, ParseError>
        +extract_content(start_pos: usize, from_container_end: bool, finished: bool) Result<crate::Event, crate::ParseError>
    }
    StreamContentBuilder ..|> ContentExtractor

    %% Event and ParseError are now referenced as crate::Event and crate::ParseError in trait signatures
Loading

Class diagram for public API visibility changes in lib.rs

classDiagram
    class lib {
        +Event
        +PullParser
        %% -EscapeTiming (no longer public)
        %% -ParserCore (no longer public)
    }
    %% EscapeTiming and ParserCore are now internal and not re-exported
Loading

File-Level Changes

Change Details Files
Refactor ContentExtractor trait to use crate-qualified types
  • Switch return types in trait signature to crate::Event and crate::ParseError
  • Update related method signatures accordingly
picojson/src/event_processor.rs
Remove obsolete is_finished method
  • Delete default is_finished implementation from the trait
  • Remove is_finished override in StreamContentBuilder
picojson/src/event_processor.rs
picojson/src/stream_content_builder.rs
Hide internal types from public API
  • Remove pub use of EscapeTiming and ParserCore in lib.rs
picojson/src/lib.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

@coderabbitai

coderabbitai Bot commented Jul 16, 2025

Copy link
Copy Markdown

Warning

Rate limit exceeded

@kaidokert has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 51 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 82059d3 and 4d22ad9.

📒 Files selected for processing (1)
  • picojson/Cargo.toml (2 hunks)

Walkthrough

The changes remove the is_finished method from the ContentExtractor trait and its implementation in StreamContentBuilder, update an import style and usage in slice_content_builder.rs, eliminate the public re-export of EscapeTiming and ParserCore from the crate root, rename and consolidate feature flags related to remote test suites, and update the GitHub Actions workflow and .gitignore accordingly. No other logic or functionality is altered.

Changes

File(s) Change Summary
picojson/src/event_processor.rs Removed is_finished method from ContentExtractor trait.
picojson/src/stream_content_builder.rs Removed is_finished implementation from ContentExtractor for StreamContentBuilder.
picojson/src/lib.rs Removed public re-export of EscapeTiming and ParserCore from crate root.
picojson/src/slice_content_builder.rs Changed import of UnicodeEscapeCollector to grouped import; updated call to process_unicode_escape_sequence to use module alias.
picojson/build.rs, picojson/tests/json_checker_tests.rs Renamed feature flag from "json-checker-tests" to "remote-tests" for conditional compilation of JSON checker test functions and modules.
.github/workflows/build.yaml Consolidated JSONTestSuite and JSON_checker tests into a single test step using remote-tests feature; removed separate json-checker-tests invocation.
.gitignore Added entries to ignore picojson/tests/data/json_checker/, picojson/tests/data/JSONTestSuite/, and picojson/tests/conformance_generated.rs.
picojson/Cargo.toml Renamed comment header from "Remote conformance testing" to "Remote conformance test suites"; removed commented-out json-checker-tests feature line.

Possibly related PRs

  • kaidokert/picojson-rs#55: Introduces the ContentExtractor trait and related event processing abstractions but does not modify or remove the is_finished method.
  • kaidokert/picojson-rs#62: Introduces the ContentExtractor trait with the is_finished method and its implementation for StreamContentBuilder, which are removed in this PR.

Poem

A trait once checked if it was done,
But now that method’s on the run!
Paths are tidy, exports lean,
Picojson’s code is fresh and clean.
🐇✨ No more “finished”—we hop along,
With simpler traits and code made strong!


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary of Changes

Hello @kaidokert, 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 focuses on refining the public API surface of the picojson crate. The primary goal is to prevent internal implementation details from being exposed externally, thereby cleaning up "API leaks" and ensuring a more controlled and stable public interface for consumers of the library.

Highlights

  • API Surface Reduction: I've made EscapeTiming and ParserCore internal components by removing their public re-export from the main lib.rs module. This prevents internal implementation details from being exposed externally.
  • Trait Refinement: I've removed the is_finished method from the ContentExtractor trait and its corresponding implementation in StreamContentBuilder. This method was an internal helper and is no longer part of the public trait contract.
  • Internal Path Correction: I've explicitly qualified Event and ParseError types with crate:: in the ContentExtractor trait. This ensures they correctly refer to the public types exposed by the crate root, improving clarity and preventing potential type resolution ambiguities.
Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and 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 to provide feedback.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @kaidokert - I've reviewed your changes and they look great!


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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request does a good job of cleaning up the public API by making internal types like ParserCore and EscapeTiming private, and by removing the is_finished method from the internal ContentExtractor trait. These changes improve encapsulation and clarify the API boundary.

I've found one point of inconsistency where the return types in the ContentExtractor trait are not all updated to use fully qualified paths (e.g., crate::Event). While the change is correctly applied to one method, it should be extended to others for consistency, as mentioned in the PR description. Please see my detailed comment.

Comment thread picojson/src/event_processor.rs Outdated
@kaidokert
kaidokert merged commit b0d34d8 into main Jul 16, 2025
@kaidokert
kaidokert deleted the cleanapi branch July 16, 2025 15:17
@coderabbitai coderabbitai Bot mentioned this pull request Aug 4, 2025
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.

1 participant