Adapt New Rules To Old Rules

[wanlwanl]

Background

• ESLint based TSV is not ready, the main-branch TSV will be replaced by ESLint based TSV in the future.

Problems

• Current tool/CI lacks of approaches to validate emitter's options and parameters in tspconfig.yaml. Sometimes, PR authors fails to add correct options for emitters. e.g. 1. SDK automation will fail 2. JS emitter has multiple types of clients (HLC/RLC/Modular), each types requires different options with specific values, if PR authors dismatch some options for another type of client, the SDK automation may generate successfully, but the generated code is incorrect and useless.

• We have monitored spec PRs for several weeks, and found many SDK automations are suffering from this issue. For now, we have to verify the tspconfig manually for problematic PR. Since some vacations will make a whole team OOF, and the manual answers will be unavailable in these periods, and the document to guide configure emitter options is also not ready.

• When the entire team who responsible for answering questions for failed SDK automation are OOF. And only one dev from another team will help to monitor the sdk automation and answer questions. The FTE may not know all the details about the configurations. Then it's a challenge to resolve service team's sdk automation failure problems for this dev.

Use Case

The added rules is used to assist reviewers/monitoring guys making sure the tspconfig is correct before furthur troubleshooting when answering questions raised from sepc authors when SDK automation fails. Spec authors are not the audience of this PR, since the messages are warning messages. In typical PR review, spec authors don't get these messages easily (assume that few spec author will check successful checks) since the check result is success even though the validation is failed.

Solution

• Implement the rules in ESLint pattern, convert the ESLint-pattern rules to current main-branch rules, in order to reduce future work.
• The new rules is defined as error/problem, etc. which will be used for long term, but the converted rules will only output warning messages for short term, in order to avoid blocking current review flow. The warning messages are mainly used by the FTE who is reponsible for answering questions for failed sdk automation, and provide an unique action to let service team to correct tspconfig.yaml for each rule.

[mikeharder]

The conversion to ESLint is not ready to merge to main, and is already being tracked by this PR:

#31909

I don't think it's worth the complexity of supporting both versions of the tool at the same time in main. If you'd like to add rules before the ESLint conversion is done, you can add them to the existing implementation in main, and I can convert the rules to ESLint along with the rest, when I complete 31909.

[kurtzeborn]

I don't think this PR should be merged for two reasons:

1. As Mike commented (and communicated previously in chat) we don't want to support rules in more than one tool. Right now they are supported in main with TypeSpecValidation. While Mike has shared the plans with you that we want to switch everything over to ESLint (and he's investigating that now), we're not 100% sure it's going to work. When he proves it does, he will be responsible for migrating all existing rules in TypeSpecValidation in main to ESLint. It will be a one-time switchover and will continue to only support a single mechanism for validation.
2. As you said, a long holiday is coming up. When our team is about to be out-of-office for an extended period of time (like Christmas in the US) we try to minimize the amount of change and code churn that occurs so that things don't break while we're out and say broken for a long time until we can give them attention. This is a big change. I'd suggest that it be held until you return from holiday. Is there any pressing need or a customer waiting on this check that necessitates getting it in right now this week?

[wanlwanl]

The conversion to ESLint is not ready to merge to main, and is already being tracked by this PR:

#31909

I don't think it's worth the complexity of supporting both versions of the tool at the same time in main. If you'd like to add rules before the ESLint conversion is done, you can add them to the existing implementation in main, and I can convert the rules to ESLint along with the rest, when I complete 31909.

I didn't use fancy ESLint functionalities, the most important and reusable part of this PR is using the ESLint's RuleModule inerface to define the rule. If you didn't like it, how about I put the code inside current TypeSpecValidation for now and keep the current implementation, then there's no new tool, and it can be resued easily when you complete #31909.

I don't think this PR should be merged for two reasons:

1. As Mike commented (and communicated previously in chat) we don't want to support rules in more than one tool. Right now they are supported in main with TypeSpecValidation. While Mike has shared the plans with you that we want to switch everything over to ESLint (and he's investigating that now), we're not 100% sure it's going to work. When he proves it does, he will be responsible for migrating all existing rules in TypeSpecValidation in main to ESLint. It will be a one-time switchover and will continue to only support a single mechanism for validation.
2. As you said, a long holiday is coming up. When our team is about to be out-of-office for an extended period of time (like Christmas in the US) we try to minimize the amount of change and code churn that occurs so that things don't break while we're out and say broken for a long time until we can give them attention. This is a big change. I'd suggest that it be held until you return from holiday. Is there any pressing need or a customer waiting on this check that necessitates getting it in right now this week?

1. I may lack of background about the overall plan to switching TypeSpecValidation to ESLint based tool, I only know that part of it is due to supporting suppressing single rules. Then I think this PR also shows it works for suppressing single rules. And this PR doesn't enable the ESLint tool, so eventually, the rules actually used is still the rules in current TypeSpecValidation check. It's still a single mechanism for validation.
2. I have consider that. To make the impact minimal: First, ESLint based TSV is disabled for now. Second, This PR won't cause the TypeSpecValidation failed, it only provide warning messages. I can add try-catch block to ensure exceptions will not throw in 100 percents. If we don't add this rules, service team constantly wrongly configured tspconfig. e.g. [MONITOR-SPEC-PR] In the Python automation pipeline, the command Push main to azure-sdk/azure-sdk-for-python:main was executed and failed azure-sdk-tools#9675
   You can try to modify multiple tspconfigs in test repo like test tsv test-repo-billy/azure-rest-api-specs#3611, it should be no surprise that many services's tspconfig is incorrect in different ways. Since Shanghai team will be OOF for next week, there's few resources to help service team correct tspconfig. So yes, it's a pressing need for this. @lirenhe, could you share more context about it?

[lirenhe]

It seems most of the rule does not have 'extraExplaination' property which describes how to fix the problem.
Is it easy for users to understand how to fix the problem for those cases?

[wanlwanl]

Yes, the rules without extraExplanation already straight forward enough. extraExplanation is used explain complex expectedValue. Below is the output of running a rule validation:

  Executing rule: tspconfig-ts-mgmt-modular-generate-metadata-true
  Validation failed.
  Error: 'options.@azure-tools/typespec-ts.generateMetadata' is NOT set to 'true' in tspconfig.yaml.
  Action: Set 'options.@azure-tools/typespec-ts.generateMetadata' to 'true' in tspconfig.yaml.
  Example:
  '''
  options:
    "@azure-tools/typespec-ts":
      generateMetadata: true
  '''

Additionally, the output will contains the action to correct the option and the sample of the option.

[mentat9]

@wanlwanl - This doesn't appear related to any ARM APIs, so I'm skipping ARM review. Removing WaitForARMFeedback to get it out of the queue. LMK, if you need anything from ARM reviewers.

[mikeharder]

Converting to draft until ESLint conversion is ready to merge to main