Display path in `TypeFormatChanged` message

[konrad-jamrozik]

Addresses:

• https://github.com/Azure/azure-sdk-tools/issues/7182

Example of error message after changes (new text emboldened):

The new version has a different format 'int64' than the previous one 'int32'. Previous path: 'old/misc_checks_01.json#/definitions/Database/properties/c'. Current path: 'new/misc_checks_01.json#/definitions/Database/properties/c'.

@mikekistler should I make it so that all errors messages always show previous/current paths?

I noticed we already display stuff like this:

was the Old / New not enough?

Implementation note:

The Old / New is populated by azureSwaggerValidation/src/unifiedPipelineHelper.ts/oadMessagesToResultMessageRecords:

  oadMessagesToResultMessageRecords(
    messages: OadMessage[],
    baseBranchName: string | null = null
  ): ResultMessageRecord[] {
    return messages.map((oadMessage) => {
      const paths: JsonPath[] = [];
      if (oadMessage.new.location) {
        paths.push({
          tag: "New",
          path: sourceBranchHref(this.context, oadMessage.new.location || ""),
        });
      }
      if (oadMessage.old.location) {
        paths.push({
          tag: "Old",
          path: specificBranchHref(
            this.context,
            oadMessage.old.location || "",
            baseBranchName || defaultBaseBranch
          ),
        });
      }

where messages: OadMessage[] come from:

breaking-change.ts / doBreakingChangeDetection:

      const filteredOadMessages = (
        option.isCrossVersion
          ? ruleManager.handleCrossApiVersion(oadMessages, ctx)
          : ruleManager.handleSameApiVersion(oadMessages, ctx)
      ).map(postHandler);

where filteredOadMessages are derived from breaking-change.ts / runOad

oadCompareOutput = await oad.compare/* variations */
// Fix up output from OAD as it does not output valid JSON.
// Specifically, replace e.g. "}  {", with "},{".
oadCompareOutput = oadCompareOutput.replace(/}\s+{/gi, "},{");

let oadMessages = JSON.parse(oadCompareOutput) as OadMessage[];

[mikeharder]

should I make it so that all errors messages always show previous/current paths?

My vote would be "yes", depending how difficult it would be to implement, and how many rules implement their own version of it already. More standardization of rules is better.

[konrad-jamrozik]

should I make it so that all errors messages always show previous/current paths?

My vote would be "yes", depending how difficult it would be to implement, and how many rules implement their own version of it already. More standardization of rules is better.

@mikeharder OK let's see how we like how this rule prints out once deployed. Based on that I will adjust all the rules.

[konrad-jamrozik]

OK looking at the code, what openapi-alps receives from openapi-diff is this data structure built from ComparisonMessage:

        public string GetValidationMessagesAsJson()
        {
            var rawMessage = new JsonComparisonMessage
            {
                id = Id.ToString(),
                code = Code.ToString(),
                message = Message,
                type = Severity.ToString(),
                docUrl = DocUrl.ToString(),
                mode = Mode.ToString(),
                old = new JsonLocation
                {
                    @ref = OldJsonRef,
                    path = OldJson()?.Path,
                    location = OldLocation(),
                },
                @new = new JsonLocation
                {
                    @ref = NewJsonRef,
                    path = NewJson()?.Path,
                    location = NewLocation(),
                }
            };

and then openapi-alps proceeds to output location property in the table rows

new JsonLocation
            {
                @ref = error.NewJsonRef,
                path = error.NewJson()?.Path,
                location = error.NewLocation(),
            };

like e.g. this one:

but I could instead make openapi-alps display different value than location. Here is example data injected into openapi-diff unit test:

The location is what we have currently, but I could replace it with path or @ref. Or I could concatenate location and path.

[konrad-jamrozik]

I am closing this PR in favor of:

• Pull Request 557572: Print out JSON paths for breaking changes table rows