General tool for combining and verifying type resolution dictionaries

[bettinaheim]

Currently, the implementation requires that for a cycle, the verification is invoked for each node. I'll see if I can find some time to be more clever about it. As a first thing to work with, this should do however, and also gives a good base line to test.

[ScottCarda-MS]

Getting to be a lot of comments, so I thought I would break it up a bit. I still need to look over the inner details of the TryCombineTypeResolutions method and the unit tests.

A lot of them are small things, or things that I can do I my end after the PR is in, if you are eager to get the PR. If there is any comment you'd rather I address after the fact, just put a response saying as much.

Once thing in particular I am hoping we can change is the signature for TryCombineTypeResolutions. I know we tend to use the Try-method pattern a lot in this code base, and sometime it is especially convenient, but using out parameters is generally to be avoided if possible, especially if the method also has a variable number of parameters as the TryCombineTypeResolutions does.

Here are two resources to consider when considering out-parameters:

• Avoid Out Parameters: https://docs.microsoft.com/en-us/previous-versions/dotnet/netframework-2.0/ms229053(v=vs.80)
• Out Parameters Best Practices: https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/parameter-design

[bettinaheim]

Getting to be a lot of comments, so I thought I would break it up a bit. I still need to look over the inner details of the TryCombineTypeResolutions method and the unit tests.
A lot of them are small things, or things that I can do I my end after the PR is in, if you are eager to get the PR. If there is any comment you'd rather I address after the fact, just put a response saying as much.
Once thing in particular I am hoping we can change is the signature for TryCombineTypeResolutions. I know we tend to use the Try-method pattern a lot in this code base, and sometime it is especially convenient, but using out parameters is generally to be avoided if possible, especially if the method also has a variable number of parameters as the TryCombineTypeResolutions does.
Here are two resources to consider when considering out-parameters:

Avoid Out Parameters: https://docs.microsoft.com/en-us/previous-versions/dotnet/netframework-2.0/ms229053(v=vs.80)
Out Parameters Best Practices: https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/parameter-design

Ah, that's a tough one. What is your concrete suggestion? Instead having the signature be
public static (bool, ImmutableDictionary<Tuple<QsQualifiedName, NonNullable<string>>, ResolvedType>) TryCombineTypeResolutions (QsQualifiedName parent, params ImmutableDictionary<Tuple<QsQualifiedName, NonNullable<string>>, ResolvedType>[] resolutions)?
It just seem weird to me since all the C# methods like TryParse, TryGetUri, Try* follow the pattern of returning a boolean and giving the value - if any - as out parameter. I would also argue that implementing this method is not exactly "designing for a general audience", given its purpose.

[ScottCarda-MS]

Some of the logic for the TryCombineTypeResolutions method is still a bit confusing to me. I am hoping that once you address some of the comments I left, it will be clearer to me.