Add `ThatAre[Not]Structs` in `TypeSelector.cs` #2056

94sedighi · 2022-12-05T09:08:16Z

IMPORTANT

The code complies with the Coding Guidelines for C#.
The changes are covered by unit tests which follow the Arrange-Act-Assert syntax and the naming conventions such as is used in these tests.
If the PR adds a feature or fixes a bug, please update the release notes with a functional description that explains what the change means to consumers of this library, which are published on the website.
If the PR changes the public API the changes needs to be included by running AcceptApiChanges.ps1 or AcceptApiChanges.sh.

…tassertions#645

docs/_pages/releases.md

Co-authored-by: IT-VBFK <49762557+IT-VBFK@users.noreply.github.com>

…TypeSelectorStructFeature

coveralls · 2022-12-08T15:57:22Z

Pull Request Test Coverage Report for Build 3731314363

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.9%) to 96.898%

Totals
Change from base Build 3730045467:	0.9%
Covered Lines:	12478
Relevant Lines:	12725

💛 - Coveralls

Src/FluentAssertions/Types/TypeSelector.cs

dennisdoomen · 2022-12-18T08:43:19Z

Tests/FluentAssertions.Specs/Types/TypeSelectorSpecs.cs

@@ -495,6 +496,42 @@ public void When_selecting_types_that_are_not_classes_it_should_return_the_corre
                .And.Contain(typeof(NotOnlyClassesEnumeration));
        }

+        [Fact]
+        public void When_selecting_types_that_are_structs_it_should_return_the_correct_types()


🤔 What about record structs?

i will extend the tests for record structs and i think it should work fine with record structs as well.

Src/FluentAssertions/Types/TypeSelector.cs

dennisdoomen · 2022-12-19T12:59:17Z

docs/_pages/releases.md

@@ -11,8 +11,11 @@ sidebar:

 ### What's new
 * Added `ThatAre[Not]Abstract` method for filtering the types - [#2058](https://github.com/fluentassertions/fluentassertions/pull/2058)
-* Added `ThatAre[Not]Abstract` methods to `MethodInfoSelector.cs` for filtering the methods - [#2060](https://github.com/fluentassertions/fluentassertions/pull/2060)
+* Added `ThatAre[Not]Struct` method for Types Filter  - [#2056](https://github.com/fluentassertions/fluentassertions/pull/2056)


jnyrup · 2022-12-19T16:50:06Z

From my comment in #2061 (comment)

E.g. in #2056 you're excluding decimal for the check if a Type is to be considered a struct.
I don't immediately know why that is, but it looks like a concern that could influence whether an API should be added and hence should be discussed in the issue instead of a PR.

In IL there is no direct concept the C# keyword struct but only value types which also includes structs.
https://learn.microsoft.com/en-us/dotnet/standard/base-types/common-type-system?redirectedfrom=MSDN#structures

public struct Foo{}

.class public sequential ansi sealed beforefieldinit Foo
    extends [System.Runtime]System.ValueType
{
    .pack 0
    .size 1

} // end of class Foo

SharpLab

So like IsCSharpAbstract and IsCSharpSealed we cannot exactly detect a 1:1 mapping between C# code written and the IL we have available.
But I think this case is more complicated, and I'm not convinced we should add this API.

Some points of the implementation

Primitive types are not considered structs despite e.g. float being an alias for the struct System.Single
System.Decimal is specially treated as a non-struct - I can only assume to behave like the other primitive types?
The newly added System.Half does not have special treatment. I assume because System.Half was added in .NET 5 after https://stackoverflow.com/a/62727198 was written?
- This is a simple case, but then how about any other custom numeric type? E.g. System.Numerics
- SharpLab

To summarize:
There are several pitfalls which makes me think this API does not pull its own weight.
Not to sound (too) grumpy, but is why I like to discuss APIs in issues before rushing to implementation.

94sedighi · 2022-12-22T14:09:16Z

I see, there is really a problem with the half and complex types.
I totally agree and will close the PR

dennisdoomen · 2022-12-23T08:13:11Z

On the other hand, a lot of developers may not even be aware of all those special cases and just want to know which of their types in their code base are struct. They will never expect to find primitive types, decimal or Half in the results, so they will never run into those specific edge cases. So I still see value in having this method, provided that we add clear documentation on these edge cases.

94sedighi · 2022-12-23T10:41:41Z

On the other hand, a lot of developers may not even be aware of all those special cases and just want to know which of their types in their code base are struct. They will never expect to find primitive types, decimal or Half in the results, so they will never run into those specific edge cases. So I still see value in having this method, provided that we add clear documentation on these edge cases.

You are right, i also didn't know about these special types that C# offer, but to be honest i'm also new in C# .NET technologies and coming from java.

jnyrup · 2022-12-23T12:20:15Z

They will never expect to find primitive types, decimal or Half in the results

As we don't know all future or custom numeric types, we cannot filter them out.

To avoid any ambiguities we could have ThatAreValueTypes.

dennisdoomen · 2022-12-24T17:01:18Z

To avoid any the ambiguities we could have ThatAreValueTypes.

Yes, let's do that. @94sedighi what do you think?

94sedighi · 2023-01-02T07:43:30Z

To avoid any the ambiguities we could have ThatAreValueTypes.

Yes, let's do that. @94sedighi what do you think?

@dennisdoomen It will cover a wider range of types, but we can do it, and there are no edge cases, as far as I know the C#.

Add ThatAre[Not]Struct in TypeSelector.cs as mentioned in Issue fluen…

b1162dd

…tassertions#645

94sedighi changed the title ~~Add ThatAre[Not]Struct in TypeSelector.cs as mentioned in Issue #645~~ Add ThatAre[Not]Struct in TypeSelector.cs as mentioned in Issue #645 Dec 5, 2022

94sedighi added 2 commits December 5, 2022 15:42

realese notes and docs have been updated

2a20180

changes of documentation

21167da

94sedighi marked this pull request as ready for review December 5, 2022 15:47

IT-VBFK suggested changes Dec 5, 2022

View reviewed changes

docs/_pages/releases.md Outdated Show resolved Hide resolved

jnyrup mentioned this pull request Dec 5, 2022

Idea / Feature #2061

Closed

8 tasks

Update docs/_pages/releases.md

53e2ab4

Co-authored-by: IT-VBFK <49762557+IT-VBFK@users.noreply.github.com>

94sedighi changed the title ~~Add ThatAre[Not]Struct in TypeSelector.cs as mentioned in Issue #645~~ Add ThatAre[Not]Struct in TypeSelector.cs Dec 6, 2022

94sedighi added 2 commits December 6, 2022 14:16

change

be52b3a

Merge remote-tracking branch 'origin/TypeSelectorStructFeature' into …

ca2a47c

…TypeSelectorStructFeature

jnyrup added the feature label Dec 12, 2022

Merge branch 'develop' into TypeSelectorStructFeature

b8a25fb

dennisdoomen requested changes Dec 18, 2022

View reviewed changes

IT-VBFK reviewed Dec 19, 2022

View reviewed changes

Src/FluentAssertions/Types/TypeSelector.cs Outdated Show resolved Hide resolved

94sedighi changed the title ~~Add ThatAre[Not]Struct in TypeSelector.cs~~ Add ThatAre[Not]Structs in TypeSelector.cs Dec 19, 2022

94sedighi added 2 commits December 19, 2022 13:09

Extended Tests and resolved conflicts

eab041b

Resolved conflicts of realease.md

5d5b73e

dennisdoomen requested changes Dec 19, 2022

View reviewed changes

94sedighi added 2 commits December 19, 2022 14:15

Update docs

76f4d06

Resolved conflicts

97581a7

jnyrup mentioned this pull request Dec 19, 2022

More Assertions on types #645

Closed

88 tasks

94sedighi closed this Dec 22, 2022

dennisdoomen reopened this Dec 24, 2022

94sedighi closed this Jan 2, 2023

94sedighi deleted the TypeSelectorStructFeature branch January 2, 2023 11:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `ThatAre[Not]Structs` in `TypeSelector.cs` #2056

Add `ThatAre[Not]Structs` in `TypeSelector.cs` #2056

94sedighi commented Dec 5, 2022 •

edited

coveralls commented Dec 8, 2022 •

edited

dennisdoomen Dec 18, 2022

94sedighi Dec 19, 2022

dennisdoomen Dec 19, 2022

jnyrup commented Dec 19, 2022

94sedighi commented Dec 22, 2022

dennisdoomen commented Dec 23, 2022

94sedighi commented Dec 23, 2022

jnyrup commented Dec 23, 2022 •

edited

dennisdoomen commented Dec 24, 2022

94sedighi commented Jan 2, 2023

Add ThatAre[Not]Structs in TypeSelector.cs #2056

Add ThatAre[Not]Structs in TypeSelector.cs #2056

Conversation

94sedighi commented Dec 5, 2022 • edited

IMPORTANT

coveralls commented Dec 8, 2022 • edited

Pull Request Test Coverage Report for Build 3731314363

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

dennisdoomen Dec 18, 2022

Choose a reason for hiding this comment

94sedighi Dec 19, 2022

Choose a reason for hiding this comment

dennisdoomen Dec 19, 2022

Choose a reason for hiding this comment

jnyrup commented Dec 19, 2022

94sedighi commented Dec 22, 2022

dennisdoomen commented Dec 23, 2022

94sedighi commented Dec 23, 2022

jnyrup commented Dec 23, 2022 • edited

dennisdoomen commented Dec 24, 2022

94sedighi commented Jan 2, 2023

Add `ThatAre[Not]Structs` in `TypeSelector.cs` #2056

Add `ThatAre[Not]Structs` in `TypeSelector.cs` #2056

94sedighi commented Dec 5, 2022 •

edited

coveralls commented Dec 8, 2022 •

edited

jnyrup commented Dec 23, 2022 •

edited