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

C#: Add flow test cases for undetected value flow, when making variable bindings in pattern matching. #7809

Merged

Conversation

michaelnebel
Copy link
Contributor

No description provided.

@michaelnebel michaelnebel requested a review from a team as a code owner February 2, 2022 10:26
@github-actions github-actions bot added the C# label Feb 2, 2022
@tamasvajk
Copy link
Contributor

A while back I also collected some unhandled tuple related patterns, but I never actually got to merging it: #5429. Maybe we could merge these efforts now.

Also, I think your test file shouldn't be in the library-tests/dataflow/fields/ folder. Maybe a new records folder would be more appropriate.

@michaelnebel
Copy link
Contributor Author

michaelnebel commented Feb 2, 2022

That is a good point!
Not sure, what the best strategy is for structuring the tests.
Even though records are used here (it could just as well had been classes or structs), it is only the case, because it was the shortest form of type declarations I could think of, where we could make use of pattern matching with property patterns and variable bindings.
Maybe we need a folder pattern for all pattern related tests instead of scattering them across the other test topics?

@michaelnebel
Copy link
Contributor Author

Should we just stick to the existing strategy?
Then I will create a records folder and move the pattern test here.

@tamasvajk
Copy link
Contributor

Should we just stick to the existing strategy? Then I will create a records folder and move the pattern test here.

I don't know if there's an explicit existing strategy here. The field folder seems to contain tests for flows through fields. So these pattern related tests don't seem to belong there. We could add them to the local folder, or to a new pattern folder. I would probably go with the latter just to be explicit. (Although the tuples folder already contains test cases with pattern matching.) What do you think @hvitved?

@hvitved
Copy link
Contributor

hvitved commented Feb 3, 2022

Let's add this to a new patterns folder (along with a new test).

@michaelnebel michaelnebel force-pushed the csharp/test-pattern-match-flow branch from df0a1f3 to ade119f Compare February 4, 2022 11:58
@michaelnebel michaelnebel merged commit f478bf5 into github:main Feb 7, 2022
@michaelnebel michaelnebel deleted the csharp/test-pattern-match-flow branch February 7, 2022 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants