Repository navigation
Use enum for diagnostic subcategories - #15236
Open
DustinCampbell wants to merge 5 commits into
Open
DustinCampbell wants to merge 5 commits into
DustinCampbell wants to merge 5 commits into
Conversation
Add DiagnosticSubcategory and its GetDisplayString extension to Microsoft.Build.Framework. Move solution-parsing and schema-validation subcategory resources, including all existing translations, into Framework's shared resources.
Replace subcategory resource-name parameters in ProjectErrorUtilities and ProjectFileErrorUtilities with DiagnosticSubcategory values. Resolve localized text through GetDisplayString and use None for uncategorized errors. Update solution parsing and graph-building callers and XML documentation.
Move ProjectErrorUtilities into Microsoft.Build under Microsoft.Build.Internal and merge ProjectFileErrorUtilities into it. Delete both linked utility files from src/Shared and update the project compile items. Add nullable signatures, control-flow annotations, and fixed-arity overloads for diagnostic subcategories, inner exceptions, and format arguments. Avoid boxing and argument-array allocations on successful verification paths and small throwing paths. Update all engine callers, replace unconditional verification calls, and add focused tests and documentation.
Replace internal logging and BuildCheck subcategory resource-name strings with DiagnosticSubcategory values. Resolve localized display text centrally in EventsCreatorHelper when creating error and warning events. Update logging services, contexts, engine callers, BuildCheck dispatch, mocks, tests, and XML documentation while preserving emitted BuildEventArgs and binary log content.
Resolve the schema-validation diagnostic subcategory through DiagnosticSubcategory instead of looking up its resource key directly. Replace misleading resource-key strings in build event test fixtures with an arbitrary subcategory display value.
Contributor
🔍 Skill Validator Results✅ All checks passed
Summary
Full validator output```text Found 1 skill(s) [authoring-errors-and-warnings] 📊 authoring-errors-and-warnings: 1,300 BPE tokens [chars/4: 1,412] (detailed ✓), 13 sections, 4 code blocks ✅ All checks passed (1 skill(s)) ``` |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The cross-assembly logging and resource migration has broad diagnostic impact, and the stated full build and test validation remains pending.
0 open findings
What changed in this PR
Replaces internal diagnostic subcategory resource keys with a strongly typed enum and centralizes localized display-string resolution in Framework.
Changes:
- Adds
DiagnosticSubcategoryand consolidates project error utilities. - Updates logging, BuildCheck, solution parsing, schema validation, and tests.
- Moves localized subcategory resources into Framework.
| File | Description |
|---|---|
src/Shared/ProjectFileErrorUtilities.cs |
Removes superseded helper. |
src/Shared/ProjectErrorUtilities.cs |
Removes shared implementation. |
src/MSBuild/Resources/Strings.resx |
Moves schema subcategory resource. |
src/MSBuild/Resources/xlf/Strings.cs.xlf |
Moves Czech translation. |
src/MSBuild/Resources/xlf/Strings.de.xlf |
Moves German translation. |
src/MSBuild/Resources/xlf/Strings.es.xlf |
Moves Spanish translation. |
src/MSBuild/Resources/xlf/Strings.fr.xlf |
Moves French translation. |
src/MSBuild/Resources/xlf/Strings.it.xlf |
Moves Italian translation. |
src/MSBuild/Resources/xlf/Strings.ja.xlf |
Moves Japanese translation. |
src/MSBuild/Resources/xlf/Strings.ko.xlf |
Moves Korean translation. |
src/MSBuild/Resources/xlf/Strings.pl.xlf |
Moves Polish translation. |
src/MSBuild/Resources/xlf/Strings.pt-BR.xlf |
Moves Portuguese translation. |
src/MSBuild/Resources/xlf/Strings.ru.xlf |
Moves Russian translation. |
src/MSBuild/Resources/xlf/Strings.tr.xlf |
Moves Turkish translation. |
src/MSBuild/Resources/xlf/Strings.zh-Hans.xlf |
Moves Simplified Chinese translation. |
src/MSBuild/Resources/xlf/Strings.zh-Hant.xlf |
Moves Traditional Chinese translation. |
src/MSBuild/ProjectSchemaValidationHandler.cs |
Uses typed schema subcategory. |
src/Framework/Resources/SR.resx |
Adds centralized subcategory resources. |
src/Framework/Resources/xlf/SR.cs.xlf |
Adds Czech translations. |
src/Framework/Resources/xlf/SR.de.xlf |
Adds German translations. |
src/Framework/Resources/xlf/SR.es.xlf |
Adds Spanish translations. |
src/Framework/Resources/xlf/SR.fr.xlf |
Adds French translations. |
src/Framework/Resources/xlf/SR.it.xlf |
Adds Italian translations. |
src/Framework/Resources/xlf/SR.ja.xlf |
Adds Japanese translations. |
src/Framework/Resources/xlf/SR.ko.xlf |
Adds Korean translations. |
src/Framework/Resources/xlf/SR.pl.xlf |
Adds Polish translations. |
src/Framework/Resources/xlf/SR.pt-BR.xlf |
Adds Portuguese translations. |
src/Framework/Resources/xlf/SR.ru.xlf |
Adds Russian translations. |
src/Framework/Resources/xlf/SR.tr.xlf |
Adds Turkish translations. |
src/Framework/Resources/xlf/SR.zh-Hans.xlf |
Adds Simplified Chinese translations. |
src/Framework/Resources/xlf/SR.zh-Hant.xlf |
Adds Traditional Chinese translations. |
src/Framework/Errors/DiagnosticSubcategory.cs |
Defines the internal enum. |
src/Framework/Errors/DiagnosticSubcategoryExtensions.cs |
Maps enum values to localized text. |
src/Build/Errors/ProjectErrorUtilities.cs |
Consolidates project-error helpers and overloads. |
src/Build/Microsoft.Build.csproj |
Compiles the consolidated helper. |
src/Build/Resources/Strings.resx |
Moves solution subcategory resource. |
src/Build/Resources/xlf/Strings.cs.xlf |
Moves Czech translation. |
src/Build/Resources/xlf/Strings.de.xlf |
Moves German translation. |
src/Build/Resources/xlf/Strings.es.xlf |
Moves Spanish translation. |
src/Build/Resources/xlf/Strings.fr.xlf |
Moves French translation. |
src/Build/Resources/xlf/Strings.it.xlf |
Moves Italian translation. |
src/Build/Resources/xlf/Strings.ja.xlf |
Moves Japanese translation. |
src/Build/Resources/xlf/Strings.ko.xlf |
Moves Korean translation. |
src/Build/Resources/xlf/Strings.pl.xlf |
Moves Polish translation. |
src/Build/Resources/xlf/Strings.pt-BR.xlf |
Moves Portuguese translation. |
src/Build/Resources/xlf/Strings.ru.xlf |
Moves Russian translation. |
src/Build/Resources/xlf/Strings.tr.xlf |
Moves Turkish translation. |
src/Build/Resources/xlf/Strings.zh-Hans.xlf |
Moves Simplified Chinese translation. |
src/Build/Resources/xlf/Strings.zh-Hant.xlf |
Moves Traditional Chinese translation. |
src/Build/Utilities/XmlUtilities.cs |
Imports the internal helper namespace. |
src/Build/Utilities/EngineFileUtilities.cs |
Uses typed warning/error APIs. |
src/Build/Instance/TaskRegistry.UsingTaskInfo.cs |
Imports the internal namespace. |
src/Build/Instance/TaskRegistry.cs |
Uses None for warnings. |
src/Build/Instance/TaskFactoryEngineContext.cs |
Uses None for warnings. |
src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs |
Updates imports. |
src/Build/Instance/ProjectPropertyInstance.cs |
Imports the internal namespace. |
src/Build/Graph/ProjectGraph.cs |
Imports the internal namespace. |
src/Build/Graph/GraphBuilder.cs |
Uses typed solution errors. |
src/Build/Evaluation/PropertiesUseTracker.cs |
Uses None for warnings. |
src/Build/Evaluation/ProjectParser.cs |
Imports the internal namespace. |
src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs |
Uses consolidated error verification. |
src/Build/Evaluation/Expander.PropertyExpander.cs |
Imports the internal namespace. |
src/Build/Evaluation/Expander.MetadataExpander.cs |
Imports the internal namespace. |
src/Build/Evaluation/Expander.ItemExpander.Transforms.cs |
Imports the internal namespace. |
src/Build/Evaluation/Expander.Function.cs |
Imports the internal namespace. |
src/Build/Evaluation/Expander.cs |
Imports the internal namespace. |
src/Build/Evaluation/Evaluator.cs |
Uses typed logging subcategories. |
src/Build/Evaluation/Conditionals/Parser.cs |
Updates warning and error helpers. |
src/Build/Evaluation/Conditionals/OrExpressionNode.cs |
Updates helper namespace. |
src/Build/Evaluation/Conditionals/NumericComparisonExpressionNode.cs |
Updates helper namespace. |
src/Build/Evaluation/Conditionals/NotExpressionNode.cs |
Updates helper namespace. |
src/Build/Evaluation/Conditionals/MultipleComparisonExpressionNode.cs |
Updates helper namespace. |
src/Build/Evaluation/Conditionals/GenericExpressionNode.cs |
Updates helper namespace. |
src/Build/Evaluation/Conditionals/FunctionCallExpressionNode.cs |
Updates helper namespace. |
src/Build/Evaluation/Conditionals/AndExpressionNode.cs |
Updates helper namespace and formatting. |
src/Build/Definition/Toolset.cs |
Uses consolidated errors and typed warnings. |
src/Build/Definition/ProjectCollection.cs |
Reorders imports. |
src/Build/Definition/Project.cs |
Imports the internal namespace. |
src/Build/Construction/Solution/SolutionProjectGenerator.cs |
Converts solution diagnostics to enum values. |
src/Build/Construction/Solution/SolutionFile.cs |
Converts parsing errors to consolidated APIs. |
src/Build/Construction/Solution/ProjectInSolution.cs |
Converts nested-project errors. |
src/Build/Construction/ProjectRootElement.cs |
Uses consolidated project errors. |
src/Build/Construction/ProjectMetadataElement.cs |
Imports the internal namespace. |
src/Build/Construction/ProjectChooseElement.cs |
Imports the internal namespace. |
src/Build/BuildCheck/Infrastructure/CheckContext/ICheckContext.cs |
Changes internal dispatch contracts. |
src/Build/BuildCheck/Infrastructure/CheckContext/CheckLoggingContext.cs |
Forwards typed subcategories. |
src/Build/BuildCheck/Infrastructure/CheckContext/CheckDispatchingContext.cs |
Creates events from typed subcategories. |
src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs |
Uses None for errors. |
src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs |
Uses None for warnings. |
src/Build/BuildCheck/Infrastructure/BuildCheckCentralContext.cs |
Uses None for warnings. |
src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs |
Uses None for acquisition errors. |
src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs |
Updates errors and warnings. |
src/Build/BackEnd/Shared/EventsCreatorHelper.cs |
Centralizes event subcategory mapping. |
src/Build/BackEnd/Shared/BuildRequestConfiguration.cs |
Uses consolidated project errors. |
src/Build/BackEnd/Node/OutOfProcNode.cs |
Uses None for serialization warnings. |
src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs |
Updates resolver diagnostics. |
src/Build/BackEnd/Components/SdkResolution/SdkResolverLoader.cs |
Uses consolidated project errors. |
src/Build/BackEnd/Components/SdkResolution/OutOfProcNodeSdkResolverService.cs |
Uses typed warning subcategories. |
src/Build/BackEnd/Components/SdkResolution/CachingSdkResolverService.cs |
Uses typed warning subcategories. |
src/Build/BackEnd/Components/RequestBuilder/TaskHost.cs |
Updates warning/error calls. |
src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs |
Uses None for warnings. |
src/Build/BackEnd/Components/RequestBuilder/TargetUpToDateChecker.cs |
Imports the internal namespace. |
src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs |
Imports the internal namespace. |
src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs |
Imports the internal namespace. |
src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs |
Uses typed strict-mode warnings. |
src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs |
Updates helper namespace. |
src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs |
Uses consolidated project errors. |
src/Build/BackEnd/Components/RequestBuilder/BatchingEngine.cs |
Updates helper namespace. |
src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs |
Uses None for plugin diagnostics. |
src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs |
Converts logging APIs to enum subcategories. |
src/Build/BackEnd/Components/Logging/LoggingContext.cs |
Converts context APIs to enum subcategories. |
src/Build/BackEnd/Components/Logging/ILoggingService.cs |
Updates internal logging contract. |
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs |
Uses typed process warnings. |
src/Build/BackEnd/BuildManager/MultiThreadedStrictModeScope.cs |
Uses typed strict-mode warnings. |
src/Build/BackEnd/BuildManager/EnvironmentVariableValidator.cs |
Uses typed environment warnings. |
src/Build/BackEnd/BuildManager/BuildManager.cs |
Updates deferred diagnostics. |
src/Build.UnitTests/ProjectErrorUtilities_Tests.cs |
Adds consolidated helper coverage. |
src/Build.UnitTests/ConfigureableForwardingLogger_Tests.cs |
Uses display subcategories in fixtures. |
src/Build.UnitTests/BackEnd/NodePackets_Tests.cs |
Updates event fixtures. |
src/Build.UnitTests/BackEnd/MultiThreadedStrictMode_Tests.cs |
Updates typed warning calls. |
src/Build.UnitTests/BackEnd/MockLoggingService.cs |
Implements updated logging contract. |
src/Build.UnitTests/BackEnd/LoggingServicesLogMethod_Tests.cs |
Tests typed logging behavior. |
src/Build.UnitTests/BackEnd/LoggingContext_Tests.cs |
Updates context tests. |
src/Build.UnitTests/BackEnd/EventSourceSink_Tests.cs |
Uses display subcategories in fixtures. |
src/BuildCheck.UnitTests/BuildCheckCentralContext_Tests.cs |
Implements updated dispatch contract. |
documentation/aot/sdk-resolution.md |
Updates helper references. |
documentation/aot/aot-trimming-strategy.md |
Updates helper references. |
.github/skills/authoring-errors-and-warnings/SKILL.md |
Documents the consolidated helper. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ 24/24 dimensions clean — no findings.
Warning
Firewall blocked 4 domains
The following domains were blocked by the firewall during workflow execution:
api.github.comgithub.compatchdiff.githubusercontent.comraw.githubusercontent.com
[!TIP]
api.github.com is blocked because GitHub API access uses the built-in GitHub tools by default. Instead of adding api.github.com to network.allowed, use tools.github.mode: gh-proxy for direct pre-authenticated GitHub CLI access without requiring network access to api.github.com:
tools:
github:
mode: gh-proxySee GitHub Tools for more information on gh-proxy mode.
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "api.github.com"
- "github.com"
- "patchdiff.githubusercontent.com"
- "raw.githubusercontent.com"See Network Configuration for more information.
Generated by Expert Code Review (on open) for #15236 · copilot · gpt56 · 1.7K AIC · ⌖ 0.505 AIC · ⊞ 24.9K
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
MSBuild's internal project-error and logging APIs passed nullable string resource names
to represent diagnostic subcategories. This required callers to know the correct resource
key and duplicated resource lookup across project errors, logging, BuildCheck, and schema
validation.
This change simplifies those APIs by replacing resource-name plumbing with a small,
strongly typed
DiagnosticSubcategoryenum and a single display-string mapping. It alsomakes progress toward removing unnecessarily shared files.
Changes Made
DiagnosticSubcategoryenum and centralize its localized display-stringmapping in
Microsoft.Build.Framework.existing translations, into Framework's shared resources.
schema-validation resource-name arguments with
DiagnosticSubcategory.ProjectFileErrorUtilitiesintoProjectErrorUtilities, move the helperinto
Microsoft.Build, and remove both linked utility files fromsrc/Shared.formatting arguments, inner exceptions, and diagnostic subcategories.
TaskLoggingHelperresource-name APIs unchanged because they resolvearbitrary task-owned resources.
plain subcategory display value.
Compatibility
This is an internal simplification with no intended user-visible behavior change. It
does not change public APIs, diagnostic codes or text, localized subcategory display
strings, event serialization, binary-log formats, protocols, defaults, or target
behavior.
DiagnosticSubcategory.Nonepreserves the existing absence of a subcategory, and publictask logging APIs remain unchanged.
Testing
ProjectErrorUtilitiesexception details, subcategories,inner exceptions, formatting overloads, successful verification paths, and control-flow
annotations.
node-packet tests for the strongly typed subcategories.
build.cmd -testran successfully.