Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(versioning): add code fixes for incompatible version errors #5459

Merged

Conversation

archerzz
Copy link
Member

@archerzz archerzz commented Dec 27, 2024

  • add code fixes for incompatible version errors
  • fix inconsistent error messages
  • add unit test cases
  • remove redundant codes

resolve #4842

- add code fixes for most of incompatible version errors
- fix inconsistent error message formats
- add unit test case
- remove redudant codes

resolve microsoft#4842
@azure-sdk
Copy link
Collaborator

azure-sdk commented Dec 27, 2024

All changed packages have been documented.

  • @typespec/versioning
Show changes

@typespec/versioning - feature ✏️

add code fixes for incompatible version errors

@azure-sdk
Copy link
Collaborator

azure-sdk commented Dec 27, 2024

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs 🛝 VSCode Extension

@RodgeFu RodgeFu added the ide Issues for VS, VSCode, Monaco, etc. label Dec 27, 2024
Mingzhe Huang (from Dev Box) added 2 commits December 30, 2024 10:06
@archerzz
Copy link
Member Author

A few issues worth mentioning.

Fix Parameter Versions

I cannot insert @added(Version.V3) with a space in the following case:

@added(Versions.v2)
model Foo {}

op test(param: Foo): void;

Ideally, it should be op test(@added(Version.V2) param:Foo): void;. But actually, the fix is:

op test(@added(Version.V2)
param:Foo): void;

The reason is that the type of param is ModelProperty, not FunctionParameter. So, I don't know if the type is a model or a parameter. For models, we should insert a decorator with newline. And for parameters, we should insert a decorator with a space. To maintain maximum compatibility, I choose to insert the decorator with a new line.

Fix by Insertion

In this PR, I only provide fixes to insert @added and @removed. However, if you check those test cases, you'll realize that we could fix by deleting or replacing existing decorators. However, I didn't find an easy and safe way to delete or replace, so I choose to only insert, which in some cases will look stupid thought it still works.

@added(Versions.V3)
@added(Versions.V2) // a better fix is to replace `@added(Versions.V3)`
model Foo {};
@removed(Versions.V2)
@added(Versions.V2) // a better fix is to delete `@removed(Versions.V2)`
model Foo{};

Alternative fixes

Some error cases can have other ways to fix. But that requires more context which is not available in the validation logic, unless we do more changes. For example,

@added(Versions.v3)
model Foo {}
        
model Bar {
  foo: Foo;
}

The error is reported on Bar.foo. We can fix by adding @added(Versions.v2) to Bar.foo. Or we can add the same decorator to Bar. I choose to fix in where the error is reported to minimize the changes.

Copy link
Member

@witemple-msft witemple-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I think these codefixes may sometimes not do what the user wants, but will be right enough of the time to be valuable to our users who are using versioning but sometimes miss these added/removed decorators where they're needed.

@archerzz archerzz added this pull request to the merge queue Feb 5, 2025
Merged via the queue into microsoft:main with commit c8cfd6b Feb 5, 2025
24 checks passed
@archerzz archerzz deleted the versioning/code-fix-incompatible-versions branch February 5, 2025 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ide Issues for VS, VSCode, Monaco, etc. lib:versioning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Codefix that add missing versioning annotations
4 participants