-
-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable latest-Recommended analysis rules #142
Conversation
lahma
commented
Jul 18, 2024
- move Directory.Build.props to root
- fix some issues
* move Directory.Build.props to root * fix some issues
<NoWarn>$(NoWarn);CA1715;</NoWarn> | ||
|
||
<!-- Do not declare visible instance fields --> | ||
<NoWarn>$(NoWarn);CA1051;</NoWarn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is for the CompilationHelper then maybe I should just use properties or readonly fields.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also all structs which don't use properties but public fields.
@@ -40,7 +42,7 @@ public TextLiteral(string text, StringComparison comparisonType) | |||
} | |||
else | |||
{ | |||
ExpectedChars = ignoreCase ? [Text.ToUpper()[0], Text.ToLower()[0]] : [Text[0]]; | |||
ExpectedChars = ignoreCase ? [Text.ToUpper(CultureInfo.CurrentCulture)[0], Text.ToLower(CultureInfo.CurrentCulture)[0]] : [Text[0]]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you read the code this makes totally sense. At first I thought this was evil but this is exactly what the argument means. Good job.