[MULTIPLE_DEFAULT_TAGS] Investigate rule to determine intent and align implementation

[mikeharder]

I believe the current implementation of getAllDefaultTags() is incorrect:

avocado/src/index.ts

Lines 175 to 186 in 8ced072

	const node = walkToNode(walker, n => n.type === 'code_block')
	if (!node) {
	break
	}
	const heading = nodeHeading(node)
	if (!heading) {
	continue
	}
	if (getHeadingLiteral(heading) === 'Basic Information' && node.literal) {
	const latestDefinition = safeLoad(node.literal)
	if (latestDefinition && latestDefinition.tag) {
	tags.push(latestDefinition.tag)

1. find all code blocks
2. filter to those under markdown heading "Basic Information"
3. load code block as YAML, without checking if lang=="yaml"
4. if parsed YAML contains tag, add to list of default tags

Steps 2 and 3 are bugs. Correct:

1. find all code blocks
2. filter to those where lang.trim().toLowerCase() === "yaml"
   • excludes code blocks with non-default conditions like yaml $(tag) == 'foo' or yaml $(foo)
3. load code block as YAML
4. if parsed YAML contains tag, add to list of default tags

Impacted PRs

• New Microsoft.Resources/deployments stable API version 2025-03-01 azure-rest-api-specs#32822

[mikeharder]

CC: @timotheeguerin, @rkmanda

[mikeharder]

I now believe this is by design. This rule was added in #86, with a test case:

https://github.com/Azure/avocado/pull/86/files#diff-2b666bbdb287f2424d042a1ff8fa4dce30d8193728ebad749047f00de4314991R31-R39

Nearly identical to the readme.md failing in a recent PR:

https://github.com/Azure/azure-rest-api-specs/pull/33389/files/d8fb2598ec1afe84375facf17048925e78dc63ba..89b1e349e6185d92353726cb2aaeec2adec20bdb#diff-40457706744f9c618a0f9c87e8c0d4798509eee512a8371b02e442848b05fffbR31-R44

The spec should remove the duplicate default tags, since this is the intent of the rule.

[mikeharder]

@timotheeguerin, @rkmanda: We've gotten multiple reports of problems with this rule. Do either of you know the history of this rule? The current implementation appears to have some issues, but setting that aside, do you have more examples of what should and shouldn't be allowed, or a definitive "spec" for this?

I think we'll need to either modify or disable this rule, because it doesn't appear compatible with existing specs, which have been allowed to merge despite this rule failing, for what appears to be a long time.

If nobody knows the original intent, maybe best to disable the rule and bring it back if/when needed?

[timotheeguerin]

i have no idea sorry