Remove unused hasWarnings and warnings - #96956
Conversation
|
I reran validation at All 14 tests across the basic TypeScript build, CLI backend, and API error-handling suites pass in Webpack production mode. The removed fields are still present in current canary, and both affected source files match this PR's base there. @lukesandberg, can you please rerun CI and review this cleanup of the TypeScript API and CLI results? The old failed run no longer exposes job details, so the local checks are the current evidence. |
afe7c8e to
480da9d
Compare
Tests PassedCommit: 6a05386 |
|
@martinfrancois merging is blocked due to missing commit signatures. can you sign the commits |
Nothing reads these two fields on `TypeCheckResult`. `verifyAndRunTypeScript` has four callers: - `build/type-check.ts` reads only `inputFilesCount`, `totalFilesCount` and `incremental` - `next-test` reads only `version` - `setup-dev-bundler` reads only `version` - `next-typegen` throws the result away `warnings` is also always empty. It looks for diagnostics with the category `Warning`, but I checked, TypeScript 6.0.2 has no messages in that category. Deprecation messages use `Suggestion` instead. So on every build we filtered and formatted every diagnostic to build a list that was always empty, and that nobody read.
480da9d to
6a05386
Compare
|
Sure @lukesandberg done, thanks for the quick review! |
Fixing a bug
Nothing reads the two fields
hasWarningsandwarningsonTypeCheckResult.verifyAndRunTypeScripthas four callers:build/type-check.tsonly readsinputFilesCount,totalFilesCountandincrementalnext-testonly readsversionsetup-dev-bundleronly readsversionnext-typegenthrows the result awaywarningsis also always empty. It looks for diagnostics with theWarningcategory, but I checked, and TypeScript 6.0.2 has no messages in that category. Deprecation messages useSuggestioninstead.So on every build, we filtered and formatted every diagnostic to build a list that was always empty, and that nobody read.
So I'm suggesting removing it instead, as it is clearly not being used. I also checked the GitHub code search across all public repositories and couldn't find anyone using either of them. That aside, it also never really worked correctly, as it would always return
hasWarnings: trueeven ifwarningswas empty.No test added since the fields are being removed. Also since this is such a small change I didn't create an issue to link here, I'd be happy to file one if you prefer me to.
Thanks for the hard work and taking the time for reviewing this PR! 😊