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

DataRow regression in support of CLS compliant #3111

Closed
Evangelink opened this issue Jun 17, 2024 · 7 comments
Closed

DataRow regression in support of CLS compliant #3111

Evangelink opened this issue Jun 17, 2024 · 7 comments

Comments

@Evangelink
Copy link
Member

Hi, we just discovered when trying to upgrade to version 3.4.3 that this problem is actually not fixed.
We have a test method with following Data rows, and the build fails on non-CLS compliant warning/error.
Please add this variant to your CLS test suite, you should be able to reproduce the problem, thanks. @Evangelink

[DataRow("")]
[DataRow(null)]

Originally posted by @martinsuchan in #1740 (comment)

Similarly this is now reported as non-CLS compliant:

[TestMethod]
[DataRow("some string", 1)]
public void StringIntDataRow(string s, int i)

Note it is possible that this non-CLS compliant behavior is the cause, or it's related to #3071 - if non-compliant tests are compiled to native code in UWP projects it might cause some troubles?

Originally posted by @martinsuchan in #1740 (comment)

@Evangelink
Copy link
Member Author

Hi @martinsuchan,

Thanks for the feedback and sorry to see there is still something broken. I have been investigating the issue and I have discussed with Roslyn team because it seems we are on some dead-end on our side and sadly it seems that CLS compliant is not really supported. I'll discuss with the team to see what we could do because we will need to either keep the current breaking state (and update the notes to mention it) and possibly introduced a ClsDataRowAttribute or we will need to break some other supported scenario.

@martinsuchan
Copy link

Hi, the thing that I don't understand - there were no CLS errors with DataRows in version 3.0, but then it started in version 3.1. So I'm wondering what was the reason that caused this and if it's possible to go back to the behavior or implementation we had before?

@Evangelink
Copy link
Member Author

Hi,

I did a bug fix in 3.0.3 (if I recall correctly) for the available ctors because some were broken as part of the refactoring/breaking changes in 3.0.0 because I broken some main scenarios in the process. If I rollback, this will be fixing your case but will be breaking some others that seem to be more mainstream.

I have been discussing with compiler team and they told me that there are many bugs on compiler side related to CLS that they don't plan to be fixing.

I am looking at other solutions I could put in place but so far, the only one that seems to be working is to introduce a new attribute. Would you be willing to make changes on your code?

@martinsuchan
Copy link

martinsuchan commented Jun 17, 2024

I think that for us it will be easiest to stay on the version 3.0 for now. Introducing new ClsDataRow feels like an overkill.
One question remains if this CLS issue is related to #3071 - if it's not possible to use DataRows for standard UWP tests at all?

@Evangelink
Copy link
Member Author

I will be investigating the UWP part soon and post back the results.

Again I am sorry for the broken state I have put you in.

@Evangelink
Copy link
Member Author

I forgot to post back here that the problem on UWP is not linked to the CLS change.

@Evangelink
Copy link
Member Author

@martinsuchan I'll move forward and close this as won't fix. After discussions with compiler team it seems that all the story around CLS compliancy is something from the past.

@Evangelink Evangelink closed this as not planned Won't fix, can't repro, duplicate, stale Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants