Skip to content

Preserve Sorbet warnings in formatted srb output#971

Merged
KaanOzkan merged 1 commit into
mainfrom
ko-fix-warnings
Jul 9, 2026
Merged

Preserve Sorbet warnings in formatted srb output#971
KaanOzkan merged 1 commit into
mainfrom
ko-fix-warnings

Conversation

@KaanOzkan

Copy link
Copy Markdown
Contributor

Capture leading Sorbet stderr lines while parsing typecheck output so formatted spoom srb tc and spoom srb bump output no longer drops warnings emitted before errors.

I initially changed the existing parse_string function but it's an API used by Tapioca, opted for a new function instead.

Capture leading Sorbet stderr lines while parsing typecheck output so formatted `spoom srb tc` and `spoom srb bump` output no longer drops warnings emitted before errors.

Assisted-By: devx/eb25d669-b4ee-4e28-9b69-129b45e1e178
@KaanOzkan KaanOzkan marked this pull request as ready for review July 8, 2026 17:12
@KaanOzkan KaanOzkan requested a review from a team as a code owner July 8, 2026 17:12
Comment on lines +59 to +63
# Used when only Sorbet errors are needed and leading stderr warnings can be ignored.
#: (String output, ?error_url_base: String) -> Array[Error]
def parse_string(output, error_url_base: DEFAULT_ERROR_URL_BASE)
parse_result(output, error_url_base: error_url_base).errors
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can't we just change this one to return a ParseResult?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's what I had initially but Tapioca uses parse_string in here. Which I had to execute during https://github.com/Shopify/spoom/blob/main/bin/export-rbi. So I went with a new function instead to be backwards compatible and ship this easier.

Comment on lines 81 to +85
#: (String output) -> Array[Error]
def parse(output)
parse_result(output).errors
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here, let's just return a ParseResult directly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We could although it's public API and would break callers if there are any

@KaanOzkan KaanOzkan merged commit 9c632ec into main Jul 9, 2026
11 checks passed
@KaanOzkan KaanOzkan deleted the ko-fix-warnings branch July 9, 2026 13:16
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.

2 participants