VS-140: Fixed Error Reported by VS-140 Jira Ticket #66
Conversation
| } | ||
| } | ||
| | ||
| return builderDefinitionNodes.ToArray(); |
There was a problem hiding this comment.
I think we can just return IEnumerable<Node> or IList<>, to skip this temporary allocations.
| { | ||
| case NodeType.Builders: | ||
| { | ||
| nodesToRewrite = GetBuildersDefinitionNodes(semanticModel, expressionNode).ToArray(); |
There was a problem hiding this comment.
So ToArray was moved to a different place. The goal was to avoid unnecessary copying of List to Array, in such transient processing.
| continue; | ||
| } | ||
| | ||
| .FirstOrDefault(n => semanticModel.GetTypeInfo(n).Type.IsSupportedIMongoCollection()) }; |
There was a problem hiding this comment.
collectionNode == null logic is gone?
| static DriverVersionHelper() | ||
| { | ||
| var driverVersion = Environment.GetEnvironmentVariable("DRIVER_VERSION"); | ||
| DriverVersions = new[] { NuGetVersion.Parse(driverVersion) }; |
| } | ||
| rewriteContext.ConstantsMapper.FinalizeLiteralsRegistration(); | ||
| | ||
| //Get Node Remappings for Root Nodes |
There was a problem hiding this comment.
No need for a comment in the case where it mostly repeats the method name.
| // Skip MongoDB.Driver.Builders<T> nodes | ||
| if (node is MemberAccessExpressionSyntax memberAccessExpressionSyntax && | ||
| semanticModel.GetSymbolInfo(memberAccessExpressionSyntax).Symbol is IPropertySymbol propertyTypeSymbol && | ||
| propertyTypeSymbol.IsDefinedInMongoDriver() && |
| // limitations under the License. | ||
| | ||
| // | ||
| using System; |
There was a problem hiding this comment.
Need these usings, and '//' ?
tests/MongoDB.Analyzer.Tests.Common.TestCases/Builders/BuildersDefinitions.cs Show resolved Hide resolved
| public FilterDefinitionBuilder<User> GetFilterDefinitionBuilder() => Builders<User>.Filter; | ||
| public static FilterDefinitionBuilder<User> GetFilterDefinitionBuilderStatic() => Builders<User>.Filter; | ||
| | ||
| public BuildersDefinitions GetClass() => new BuildersDefinitions(); |
There was a problem hiding this comment.
=> new(); (and next line)
| var typeArguments = rootTypeArguments.Select(rootTypeArgument => SyntaxFactory.ParseTypeName(typesProcessor | ||
| .GetTypeSymbolToGeneratedTypeMapping(rootTypeArgument))).ToArray(); | ||
| | ||
| var buildersGenericType = SyntaxFactory.GenericName("Builders").WithTypeArgumentList( |
There was a problem hiding this comment.
Should be a const in MqlGeneratorSyntaxElements.Builders.
| var rootTypeArguments = rootType.TypeArguments.ToArray(); | ||
| | ||
| var typeArguments = rootTypeArguments.Select(rootTypeArgument => SyntaxFactory.ParseTypeName(typesProcessor | ||
| .GetTypeSymbolToGeneratedTypeMapping(rootTypeArgument))).ToArray(); |
There was a problem hiding this comment.
No need to convert to arrays at all (here and above).
Also we need to check whether type was processed by typesProcessor, and return early if it was not, like it's done in HandleRemappedType.
| if (symbolInfo.Symbol == null) | ||
| { | ||
| return default; | ||
| nodesRemapping.Clear(); |
There was a problem hiding this comment.
I am not a fan of indicating an error by clearing accumulated results. Let's have this method return a boolean indicating it's success.
| | ||
| [BuildersMQL("{ \"Name\" : \"User Name1\" }")] | ||
| [BuildersMQL("{ \"Name\" : \"User Name2\" }")] | ||
| public void MethodTest() |
There was a problem hiding this comment.
No need for 'Test' suffix. Let's try come up with a better convention here. Something like
Variable_definition_projection,
Variable_definition_indexkeys,
Method_definition
Static_method_definition
...
| expressionDescendants = expressionNode.DescendantNodes(n => n != rootNode).OfType<IdentifierNameSyntax>(); | ||
| rootNodeRemapped = SyntaxFactory.IdentifierName(MqlGeneratorSyntaxElements.Linq.QueryableName); | ||
| expressionDescendants = expressionNode.DescendantNodes(n => !rootNodes.Contains(n)).OfType<IdentifierNameSyntax>(); | ||
| processGenerics = false; |
There was a problem hiding this comment.
Node need for setting processGenerics here?
3 participants
No description provided.