Skip to content

Commit 2ac2bad

Browse files
rstamBorisDog
rstam
authored and
BorisDog
committed
CSHARP-4100: Review uses of StringComparison in StartsWith, Contains and EndsWith.
1 parent 5f7f4f8 commit 2ac2bad

File tree

9 files changed

+2224
-87
lines changed

9 files changed

+2224
-87
lines changed

‎src/MongoDB.Driver/Linq/Linq3Implementation/Reflection/StringMethod.cs

Lines changed: 43 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,14 @@ namespace MongoDB.Driver.Linq.Linq3Implementation.Misc
2323
internal static class StringMethod
2424
{
2525
// private static fields
26-
private static readonly MethodInfo __contains;
27-
#if NETSTANDARD2_1_OR_GREATER
28-
private static readonly MethodInfo __containsWithComparisonType;
29-
#endif
30-
private static readonly MethodInfo __endsWith;
31-
private static readonly MethodInfo __endsWithWithComparisonType;
32-
private static readonly MethodInfo __endsWithWithIgnoreCaseAndCulture;
26+
private static readonly MethodInfo __containsWithChar;
27+
private static readonly MethodInfo __containsWithCharAndComparisonType;
28+
private static readonly MethodInfo __containsWithString;
29+
private static readonly MethodInfo __containsWithStringAndComparisonType;
30+
private static readonly MethodInfo __endsWithWithChar;
31+
private static readonly MethodInfo __endsWithWithString;
32+
private static readonly MethodInfo __endsWithWithStringAndComparisonType;
33+
private static readonly MethodInfo __endsWithWithStringAndIgnoreCaseAndCulture;
3334
private static readonly MethodInfo __getChars;
3435
private static readonly MethodInfo __indexOfAny;
3536
private static readonly MethodInfo __indexOfAnyWithStartIndex;
@@ -53,9 +54,10 @@ internal static class StringMethod
5354
private static readonly MethodInfo __splitWithCharsAndOptions;
5455
private static readonly MethodInfo __splitWithStringsAndCountAndOptions;
5556
private static readonly MethodInfo __splitWithStringsAndOptions;
56-
private static readonly MethodInfo __startsWith;
57-
private static readonly MethodInfo __startsWithWithComparisonType;
58-
private static readonly MethodInfo __startsWithWithIgnoreCaseAndCulture;
57+
private static readonly MethodInfo __startsWithWithChar;
58+
private static readonly MethodInfo __startsWithWithString;
59+
private static readonly MethodInfo __startsWithWithStringAndComparisonType;
60+
private static readonly MethodInfo __startsWithWithStringAndIgnoreCaseAndCulture;
5961
private static readonly MethodInfo __strLenBytes;
6062
private static readonly MethodInfo __substrBytes;
6163
private static readonly MethodInfo __substring;
@@ -74,13 +76,24 @@ internal static class StringMethod
7476
// static constructor
7577
static StringMethod()
7678
{
77-
__contains = ReflectionInfo.Method((string s, string value) => s.Contains(value));
7879
#if NETSTANDARD2_1_OR_GREATER
79-
__containsWithComparisonType = ReflectionInfo.Method((string s, string value, StringComparison comparisonType) => s.Contains(value, comparisonType));
80+
__containsWithChar = ReflectionInfo.Method((string s, char value) => s.Contains(value));
81+
__containsWithCharAndComparisonType = ReflectionInfo.Method((string s, char value, StringComparison comparisonType) => s.Contains(value, comparisonType));
82+
__containsWithStringAndComparisonType = ReflectionInfo.Method((string s, string value, StringComparison comparisonType) => s.Contains(value, comparisonType));
83+
__endsWithWithChar = ReflectionInfo.Method((string s, char value) => s.EndsWith(value));
84+
__startsWithWithChar = ReflectionInfo.Method((string s, char value) => s.StartsWith(value));
85+
#else
86+
__containsWithChar = null;
87+
__containsWithCharAndComparisonType = null;
88+
__containsWithStringAndComparisonType = null;
89+
__endsWithWithChar = null;
90+
__startsWithWithChar = null;
8091
#endif
81-
__endsWith = ReflectionInfo.Method((string s, string value) => s.EndsWith(value));
82-
__endsWithWithComparisonType = ReflectionInfo.Method((string s, string value, StringComparison comparisonType) => s.EndsWith(value, comparisonType));
83-
__endsWithWithIgnoreCaseAndCulture = ReflectionInfo.Method((string s, string value, bool ignoreCase, CultureInfo culture) => s.EndsWith(value, ignoreCase, culture));
92+
93+
__containsWithString = ReflectionInfo.Method((string s, string value) => s.Contains(value));
94+
__endsWithWithString = ReflectionInfo.Method((string s, string value) => s.EndsWith(value));
95+
__endsWithWithStringAndComparisonType = ReflectionInfo.Method((string s, string value, StringComparison comparisonType) => s.EndsWith(value, comparisonType));
96+
__endsWithWithStringAndIgnoreCaseAndCulture = ReflectionInfo.Method((string s, string value, bool ignoreCase, CultureInfo culture) => s.EndsWith(value, ignoreCase, culture));
8497
__getChars = ReflectionInfo.Method((string s, int index) => s[index]);
8598
__indexOfAny = ReflectionInfo.Method((string s, char[] anyOf) => s.IndexOfAny(anyOf));
8699
__indexOfAnyWithStartIndex = ReflectionInfo.Method((string s, char[] anyOf, int startIndex) => s.IndexOfAny(anyOf, startIndex));
@@ -104,9 +117,9 @@ static StringMethod()
104117
__splitWithCharsAndOptions = ReflectionInfo.Method((string s, char[] separator, StringSplitOptions options) => s.Split(separator, options));
105118
__splitWithStringsAndCountAndOptions = ReflectionInfo.Method((string s, string[] separator, int count, StringSplitOptions options) => s.Split(separator, count, options));
106119
__splitWithStringsAndOptions = ReflectionInfo.Method((string s, string[] separator, StringSplitOptions options) => s.Split(separator, options));
107-
__startsWith = ReflectionInfo.Method((string s, string value) => s.StartsWith(value));
108-
__startsWithWithComparisonType = ReflectionInfo.Method((string s, string value, StringComparison comparisonType) => s.StartsWith(value, comparisonType));
109-
__startsWithWithIgnoreCaseAndCulture = ReflectionInfo.Method((string s, string value, bool ignoreCase, CultureInfo culture) => s.StartsWith(value, ignoreCase, culture));
120+
__startsWithWithString = ReflectionInfo.Method((string s, string value) => s.StartsWith(value));
121+
__startsWithWithStringAndComparisonType = ReflectionInfo.Method((string s, string value, StringComparison comparisonType) => s.StartsWith(value, comparisonType));
122+
__startsWithWithStringAndIgnoreCaseAndCulture = ReflectionInfo.Method((string s, string value, bool ignoreCase, CultureInfo culture) => s.StartsWith(value, ignoreCase, culture));
110123
__strLenBytes = ReflectionInfo.Method((string s) => s.StrLenBytes());
111124
__substrBytes = ReflectionInfo.Method((string s, int startIndex, int length) => s.SubstrBytes(startIndex, length));
112125
__substring = ReflectionInfo.Method((string s, int startIndex) => s.Substring(startIndex));
@@ -124,13 +137,14 @@ static StringMethod()
124137
}
125138

126139
// public properties
127-
public static MethodInfo Contains => __contains;
128-
#if NETSTANDARD2_1_OR_GREATER
129-
public static MethodInfo ContainsWithComparisonType => __containsWithComparisonType;
130-
#endif
131-
public static MethodInfo EndsWith => __endsWith;
132-
public static MethodInfo EndsWithWithComparisonType => __endsWithWithComparisonType;
133-
public static MethodInfo EndsWithWithIgnoreCaseAndCulture => __endsWithWithIgnoreCaseAndCulture;
140+
public static MethodInfo ContainsWithChar => __containsWithChar;
141+
public static MethodInfo ContainsWithCharAndComparisonType => __containsWithCharAndComparisonType;
142+
public static MethodInfo ContainsWithString => __containsWithString;
143+
public static MethodInfo ContainsWithStringAndComparisonType => __containsWithStringAndComparisonType;
144+
public static MethodInfo EndsWithWithChar => __endsWithWithChar;
145+
public static MethodInfo EndsWithWithString => __endsWithWithString;
146+
public static MethodInfo EndsWithWithStringAndComparisonType => __endsWithWithStringAndComparisonType;
147+
public static MethodInfo EndsWithWithStringAndIgnoreCaseAndCulture => __endsWithWithStringAndIgnoreCaseAndCulture;
134148
public static MethodInfo GetChars => __getChars;
135149
public static MethodInfo IndexOfAny => __indexOfAny;
136150
public static MethodInfo IndexOfAnyWithStartIndex => __indexOfAnyWithStartIndex;
@@ -154,11 +168,12 @@ static StringMethod()
154168
public static MethodInfo SplitWithCharsAndOptions => __splitWithCharsAndOptions;
155169
public static MethodInfo SplitWithStringsAndCountAndOptions => __splitWithStringsAndCountAndOptions;
156170
public static MethodInfo SplitWithStringsAndOptions => __splitWithStringsAndOptions;
157-
public static MethodInfo StartsWith => __startsWith;
158-
public static MethodInfo StartsWithWithComparisonType => __startsWithWithComparisonType;
171+
public static MethodInfo StartsWithWithChar => __startsWithWithChar;
172+
public static MethodInfo StartsWithWithString => __startsWithWithString;
173+
public static MethodInfo StartsWithWithStringAndComparisonType => __startsWithWithStringAndComparisonType;
174+
public static MethodInfo StartsWithWithStringAndIgnoreCaseAndCulture => __startsWithWithStringAndIgnoreCaseAndCulture;
159175
public static MethodInfo StrLenBytes => __strLenBytes;
160176
public static MethodInfo SubstrBytes => __substrBytes;
161-
public static MethodInfo StartsWithWithIgnoreCaseAndCulture => __startsWithWithIgnoreCaseAndCulture;
162177
public static MethodInfo Substring => __substring;
163178
public static MethodInfo SubstringWithLength => __substringWithLength;
164179
public static MethodInfo ToLower => __toLower;

‎src/MongoDB.Driver/Linq/Linq3Implementation/Serializers/KnownSerializers/KnownSerializersNode.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ private HashSet<IBsonSerializer> GetPossibleSerializersAtThisLevel(Type type)
7777
}
7878

7979
Type itemType = null;
80-
if (type.TryGetIEnumerableGenericInterface(out var ienumerableGenericInterface))
80+
if (type != typeof(string) && type.TryGetIEnumerableGenericInterface(out var ienumerableGenericInterface))
8181
{
8282
itemType = ienumerableGenericInterface.GetGenericArguments()[0];
8383
}

‎src/MongoDB.Driver/Linq/Linq3Implementation/Translators/ExpressionToAggregationExpressionTranslators/MethodTranslators/ContainsMethodToAggregationExpressionTranslator.cs

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
*/
1515

1616
using System.Linq.Expressions;
17-
using System.Reflection;
1817
using MongoDB.Bson.Serialization.Serializers;
1918
using MongoDB.Driver.Linq.Linq3Implementation.Ast.Expressions;
2019
using MongoDB.Driver.Linq.Linq3Implementation.Misc;
@@ -24,23 +23,10 @@ namespace MongoDB.Driver.Linq.Linq3Implementation.Translators.ExpressionToAggreg
2423
{
2524
internal static class ContainsMethodToAggregationExpressionTranslator
2625
{
27-
private static readonly MethodInfo[] __containsMethods;
28-
29-
static ContainsMethodToAggregationExpressionTranslator()
30-
{
31-
__containsMethods = new[]
32-
{
33-
StringMethod.Contains,
34-
#if NETSTANDARD2_1_OR_GREATER
35-
StringMethod.ContainsWithComparisonType
36-
#endif
37-
};
38-
}
39-
4026
// public methods
4127
public static AggregationExpression Translate(TranslationContext context, MethodCallExpression expression)
4228
{
43-
if (expression.Method.IsOneOf(__containsMethods))
29+
if (StartsWithContainsOrEndsWithMethodToAggregationExpressionTranslator.CanTranslate(expression))
4430
{
4531
return StartsWithContainsOrEndsWithMethodToAggregationExpressionTranslator.Translate(context, expression);
4632
}

‎src/MongoDB.Driver/Linq/Linq3Implementation/Translators/ExpressionToAggregationExpressionTranslators/MethodTranslators/StartsWithContainsOrEndsWithMethodToAggregationExpressionTranslator.cs

Lines changed: 81 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,65 +14,114 @@
1414
*/
1515

1616
using System;
17+
using System.Collections.ObjectModel;
1718
using System.Globalization;
19+
using System.Linq;
1820
using System.Linq.Expressions;
1921
using System.Reflection;
22+
using MongoDB.Bson;
23+
using MongoDB.Bson.Serialization;
2024
using MongoDB.Bson.Serialization.Serializers;
2125
using MongoDB.Driver.Linq.Linq3Implementation.Ast.Expressions;
2226
using MongoDB.Driver.Linq.Linq3Implementation.ExtensionMethods;
2327
using MongoDB.Driver.Linq.Linq3Implementation.Misc;
28+
using MongoDB.Driver.Linq.Linq3Implementation.Reflection;
2429

2530
namespace MongoDB.Driver.Linq.Linq3Implementation.Translators.ExpressionToAggregationExpressionTranslators.MethodTranslators
2631
{
2732
internal static class StartsWithContainsOrEndsWithMethodToAggregationExpressionTranslator
2833
{
29-
private static readonly MethodInfo[] __startsWithContainsOrEndWithMethods;
34+
private static readonly MethodInfo[] __startsWithContainsOrEndsWithMethods;
3035
private static readonly MethodInfo[] __withComparisonTypeMethods;
3136
private static readonly MethodInfo[] __withIgnoreCaseAndCultureMethods;
3237

3338
static StartsWithContainsOrEndsWithMethodToAggregationExpressionTranslator()
3439
{
35-
__startsWithContainsOrEndWithMethods = new[]
40+
__startsWithContainsOrEndsWithMethods = new[]
3641
{
37-
StringMethod.StartsWith,
38-
StringMethod.StartsWithWithComparisonType,
39-
StringMethod.StartsWithWithIgnoreCaseAndCulture,
40-
StringMethod.Contains,
41-
#if NETSTANDARD2_1_OR_GREATER
42-
StringMethod.ContainsWithComparisonType,
43-
#endif
44-
StringMethod.EndsWith,
45-
StringMethod.EndsWithWithComparisonType,
46-
StringMethod.EndsWithWithIgnoreCaseAndCulture
42+
StringMethod.StartsWithWithChar,
43+
StringMethod.StartsWithWithString,
44+
StringMethod.StartsWithWithStringAndComparisonType,
45+
StringMethod.StartsWithWithStringAndIgnoreCaseAndCulture,
46+
StringMethod.ContainsWithChar,
47+
StringMethod.ContainsWithCharAndComparisonType,
48+
StringMethod.ContainsWithString,
49+
StringMethod.ContainsWithStringAndComparisonType,
50+
StringMethod.EndsWithWithChar,
51+
StringMethod.EndsWithWithString,
52+
StringMethod.EndsWithWithStringAndComparisonType,
53+
StringMethod.EndsWithWithStringAndIgnoreCaseAndCulture
4754
};
4855

4956
__withComparisonTypeMethods = new[]
5057
{
51-
StringMethod.StartsWithWithComparisonType,
52-
#if NETSTANDARD2_1_OR_GREATER
53-
StringMethod.ContainsWithComparisonType,
54-
#endif
55-
StringMethod.EndsWithWithComparisonType
58+
StringMethod.StartsWithWithStringAndComparisonType,
59+
StringMethod.ContainsWithCharAndComparisonType,
60+
StringMethod.ContainsWithStringAndComparisonType,
61+
StringMethod.EndsWithWithStringAndComparisonType
5662
};
5763

5864
__withIgnoreCaseAndCultureMethods = new[]
5965
{
60-
StringMethod.StartsWithWithIgnoreCaseAndCulture,
61-
StringMethod.EndsWithWithIgnoreCaseAndCulture
66+
StringMethod.StartsWithWithStringAndIgnoreCaseAndCulture,
67+
StringMethod.EndsWithWithStringAndIgnoreCaseAndCulture
6268
};
6369
}
6470

71+
public static bool CanTranslate(MethodCallExpression expression)
72+
{
73+
var method = expression.Method;
74+
75+
if (method.Is(EnumerableMethod.Contains) && expression.Arguments[0].Type == typeof(string))
76+
{
77+
return true;
78+
}
79+
80+
return method.IsOneOf(__startsWithContainsOrEndsWithMethods);
81+
}
82+
6583
public static AggregationExpression Translate(TranslationContext context, MethodCallExpression expression)
6684
{
6785
var method = expression.Method;
6886
var arguments = expression.Arguments;
6987

70-
if (method.IsOneOf(__startsWithContainsOrEndWithMethods))
88+
if (CanTranslate(expression))
7189
{
72-
var objectExpression = expression.Object;
90+
Expression objectExpression;
91+
if (method.Is(EnumerableMethod.Contains))
92+
{
93+
objectExpression = arguments[0];
94+
arguments = new ReadOnlyCollection<Expression>(arguments.Skip(1).ToList());
95+
96+
if (objectExpression.Type != typeof(string))
97+
{
98+
throw new ExpressionNotSupportedException(objectExpression, expression, because: "type implementing IEnumerable<char> is not string");
99+
}
100+
}
101+
else
102+
{
103+
objectExpression = expression.Object;
104+
}
105+
73106
var objectTranslation = ExpressionToAggregationExpressionTranslator.Translate(context, objectExpression);
74107
var valueExpression = arguments[0];
75-
var valueTranslation = ExpressionToAggregationExpressionTranslator.Translate(context, valueExpression);
108+
AggregationExpression valueTranslation;
109+
if (valueExpression.Type == typeof(char) &&
110+
valueExpression is ConstantExpression constantValueExpression)
111+
{
112+
var c = (char)constantValueExpression.Value;
113+
var value = new string(c, 1);
114+
valueTranslation = new AggregationExpression(valueExpression, value, objectTranslation.Serializer);
115+
}
116+
else
117+
{
118+
valueTranslation = ExpressionToAggregationExpressionTranslator.Translate(context, valueExpression);
119+
if (valueTranslation.Serializer is IRepresentationConfigurable representationConfigurable &&
120+
representationConfigurable.Representation != BsonType.String)
121+
{
122+
throw new ExpressionNotSupportedException(valueExpression, expression, because: "it is not serialized as a string");
123+
}
124+
}
76125
bool ignoreCase = false;
77126
if (method.IsOneOf(__withComparisonTypeMethods))
78127
{
@@ -123,7 +172,12 @@ static AstExpression CreateEndsWithAst(AstExpression stringAst, AstExpression su
123172
var (stringVar, stringSimpleAst) = AstExpression.UseVarIfNotSimple("string", stringAst);
124173
var (substringVar, substringSimpleAst) = AstExpression.UseVarIfNotSimple("substring", substringAst);
125174
var startAst = AstExpression.Subtract(AstExpression.StrLenCP(stringSimpleAst), AstExpression.StrLenCP(substringSimpleAst));
126-
var ast = AstExpression.Gte(AstExpression.IndexOfCP(stringSimpleAst, substringSimpleAst, startAst), 0);
175+
var startVar = AstExpression.Var("start");
176+
var ast = AstExpression.Let(
177+
var: AstExpression.VarBinding(startVar, startAst),
178+
@in: AstExpression.And(
179+
AstExpression.Gte(startVar, 0),
180+
AstExpression.Eq(AstExpression.IndexOfCP(stringSimpleAst, substringSimpleAst, startVar), startVar)));
127181
return AstExpression.Let(stringVar, substringVar, ast);
128182
}
129183
}
@@ -137,20 +191,20 @@ bool GetIgnoreCaseFromComparisonType(Expression comparisonTypeExpression)
137191
case StringComparison.CurrentCultureIgnoreCase: return true;
138192
}
139193

140-
throw new ExpressionNotSupportedException(comparisonTypeExpression, expression);
194+
throw new ExpressionNotSupportedException(comparisonTypeExpression, expression, because: $"{comparisonType} is not supported");
141195
}
142196

143197
bool GetIgnoreCaseFromIgnoreCaseAndCulture(Expression ignoreCaseExpression, Expression cultureExpression)
144198
{
145199
var ignoreCase = ignoreCaseExpression.GetConstantValue<bool>(containingExpression: expression);
146200
var culture = cultureExpression.GetConstantValue<CultureInfo>(containingExpression: expression);
147201

148-
if (culture == CultureInfo.CurrentCulture)
202+
if (culture != CultureInfo.CurrentCulture)
149203
{
150-
return ignoreCase;
204+
throw new ExpressionNotSupportedException(cultureExpression, expression, because: "the supplied culture is not the current culture");
151205
}
152206

153-
throw new ExpressionNotSupportedException(cultureExpression, expression);
207+
return ignoreCase;
154208
}
155209
}
156210
}

0 commit comments

Comments
 (0)