TypeSpec Validation PR pipeline check flags issues unrelated to the current PR

[mikekistler]

The Cadl Validation PR pipeline check in the azure-rest-api-specs repo is flagging problems that are unrelated to the changes made in the current PR.

An example of this problem is PR #22773, which makes a very small change to the Cadl for Multivariate Anomaly Detector. The Cadl Validation check in this PR reports a number of issues with the Cadl for OpenAI -- a completely different service.

[mikeharder]

The issue is function getCadlRootFolder() only considers the first subfolder under the specification folder, so it analyzes all of specification/cognitiveservices, not just specification/cognitiveservices/AnomalyDetector.

[mikeharder]

I believe fixing this correctly would require parsing the import statements from *.tsp files to create a dependency graph, which I'm not sure is worth doing. Explanation follows.

Definitions

1. A "service directory" is a folder one level deep under specification, like specification/cognitiveservices.
2. A "TypeSpec project" is a folder under a service directory containing a tspconfig.yaml file.
3. A service directory may contain one or more TypeSpec projects.
4. A service directory may also contain *.tsp files outside of a TypeSpec project, that are imported by files inside one or more TypeSpec projects.

Rules

1. Imported files must be contained within the same service directory. One service directory cannot import files from a different service directory.
2. Imported files may be required to be in folder named "*.Shared"

Validation Options

1. If any *.tsp or tspconfig.yaml file is changed under a "service directory", validate all projects under the service directory.
   • Does not require parsing *.tsp files to create the dependency graph of imports.
2. Only validate projects that contain or import changed *.tsp or tspconfig.yaml files
   • Requires parsing *.tsp files to create a dependency graph of imports.

Conclusions

Option 1 is simpler but less granular, and may validate TypeSpec projects that technically didn't change in the PR (but really should be valid anyway).

Option 2 is more granular but more complex to implement (since it requires creating a dependency graph of *.tsp files).

TypeSpecValidation currently uses option #1, and I propose leaving it this way unless we are certain we prefer #2.

[mikeharder]

@mikekistler, @catalinaperalta: Please see my explanation of the options for this issue above. If we want to scope the set of TypeSpec Projects to validate smaller than the "service directory", it will require parsing import statements in *.tsp files which is doable but something we'd prefer to avoid.

Examples

1. If a change is made under https://github.com/Azure/azure-rest-api-specs/tree/main/specification/contosowidgetmanager/Contoso.WidgetManager.Shared, we need to know to validate https://github.com/Azure/azure-rest-api-specs/tree/main/specification/contosowidgetmanager/Contoso.WidgetManager.

2. If a change is made under https://github.com/Azure/azure-rest-api-specs/tree/main/specification/cognitiveservices/HealthInsights/healthinsights.trialmatcher, we need to know to validate https://github.com/Azure/azure-rest-api-specs/tree/main/specification/cognitiveservices/HealthInsights/healthinsights.openapi, since it imports from trialmatcher. But not validate anything else under specification/cognitiveservices (because there is no guarantee other projects don't also import from trialmatcher.

CC: @weshaggard

[ckairen]

This issue is fixed with the new version of TypeSpec Validation