Refactorings

[bart-degreed]

Fixes #1027.
Fixes #864.
Fixes #944.
Fixes #1033.

Motivation for (immutable) collection usage in expressions:

• AnyExpression.Constants: Set because the order is irrelevant and having duplicates is pointless.
• IncludeExpression.Elements and IncludeElementExpression.Children: List because order should be preserved. This order is used in rendered links, but more importantly, it determines the order in which related resources are written to the response body. Although JSON:API does not specify ordering for included[], API clients may have taken a dependency on the current ordering. It does not feel justified to break in this case, because there's no real benefit in doing so.
• LogicalExpression.Terms: List because the terms are short-circuited, so order matters. For example, in "A and B" when A is false, B is never evaluated.
• PaginationQueryStringValueExpression.Elements: List, because this expression type is close to the typed text in the query string. Moreover, making this a Set does not prevent duplicates, due to how equality on elements is defined. For example "articles:3" is not equal to "articles:8", so the set will contain both. Deduplication occurs when constraints are built (the last one wins).
• ResourceFieldChainExpression.Fields: List because it represents an ordered chain. "Owner.LastName" is not the same as "LastName.Owner".
• SortExpression.Elements: List because order determines sort-within-sort. For example, sort on Name first, then within that, on Age.
• SparseFieldSetExpression.Fields: Set because the order is irrelevant (we emit fields in declaration order) and having duplicates is pointless.
• SparseFieldTableExpression.Table: Dictionary, which it was already (mapping from resource type to sparse fieldset).

Motivation for not using IReadOnlySet<> (instead of ISet<>) in AddToToMany/RemoveFromToMany in pipeline:

• There's no point in artificially blocking the ability to change the set contents in-between while flowing through the pipeline. In fact, that's exactly what IResourceDefinition.On***RelationshipAsync callbacks facilitate.

QUALITY CHECKLIST

• Changes implemented in code
• Complies with our contributing guidelines
• Adapted tests
• Documentation updated
• N/A: Created issue to update Templates: {ISSUE_NUMBER}

[codecov]

Codecov Report

Merging #1038 (e760e2c) into master (3e982ec) will increase coverage by 0.28%.
The diff coverage is 91.30%.

❗ Current head e760e2c differs from pull request most recent head 0c576be. Consider uploading reports for the commit 0c576be to get more accurate results

@@            Coverage Diff             @@
##           master    #1038      +/-   ##
==========================================
+ Coverage   91.35%   91.63%   +0.28%     
==========================================
  Files         254      252       -2     
  Lines        6765     6777      +12     
==========================================
+ Hits         6180     6210      +30     
+ Misses        585      567      -18     

Impacted Files	Coverage Δ
...otNetCoreExample/Definitions/TodoItemDefinition.cs	0.00% <0.00%> (ø)
...EntityFrameworkExample/Services/WorkItemService.cs	85.00% <ø> (ø)
...ore/AtomicOperations/OperationProcessorAccessor.cs	95.00% <0.00%> (ø)
src/JsonApiDotNetCore/CollectionExtensions.cs	39.13% <0.00%> (-60.87%)	⬇️
...DotNetCore/Controllers/JsonApiCommandController.cs	0.00% <0.00%> (ø)
...Core/Queries/Expressions/QueryExpressionVisitor.cs	4.34% <ø> (ø)
...tNetCore/Queries/Internal/Parsing/IncludeParser.cs	100.00% <ø> (ø)
...nternal/Parsing/QueryStringParameterScopeParser.cs	83.33% <ø> (ø)
.../Internal/QueryableBuilding/SelectClauseBuilder.cs	97.67% <ø> (+6.27%)	⬆️
...s/Internal/QueryableBuilding/WhereClauseBuilder.cs	94.00% <ø> (ø)
... and 70 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e982ec...0c576be. Read the comment docs.