微软发布其项目的源代码是验证它们的一个很好的理由。 这次也不例外，今天我们看一下在Infer.NET代码中发现的可疑位置。 向下注解-直达重点！
关于项目和分析器的一些知识
Infer.NET是由Microsoft的专家开发的机器学习系统。 该项目的源代码最近已在
GitHub上提供 ，这就是进行验证的原因。 例如，可以
在此处找到有关该项目的更多详细信息。
使用PVS-Studio版本6.26静态分析器检查了该项目。 让我提醒您，PVS-Studio正在Windows，Linux，macOS下的C \ C ++ \ C＃（以及Java中）中寻找代码错误。 到目前为止，我们仅在Windows下分析C＃代码。 可以
下载分析仪
并在您的项目上
试用 。
检查本身非常简单，没有问题。 以前，我从GitHub上卸载了该项目，还原了所需的软件包（依赖项），并确保该项目已成功构建。 这是必需的，以便分析仪可以访问所有必要的信息以进行全面分析。 单击几下后，我通过用于Visual Studio的PVS-Studio插件启动了对解决方案的分析。
顺便说一下，这不是Microsoft使用PVS-Studio测试的第一个项目-还有其他项目：
Roslyn ，
MSBuild ，
PowerShell ，
CoreFX 等 。
注意事项 如果您或您的熟人对分析Java代码感兴趣，可以通过选择“我想要Java分析器”来给我们写信以获取
支持 。 该分析器没有公开的Beta版本，但应尽快提供。 他们在秘密实验室的某个地方（穿过墙壁）正在积极地工作。
但是足够多的抽象讨论-让我们看一下代码中的问题。
这是错误还是功能？
我建议您自己尝试查找错误-完全可以解决的任务。 老实说，我不会以“
2017年C ++项目中的十大错误 ”一文
为笑。 因此，请勿急于阅读代码段后提供的分析器警告。
private void MergeParallelTransitions() { .... if ( transition1.DestinationStateIndex == transition2.DestinationStateIndex && transition1.Group == transition2.Group) { if (transition1.IsEpsilon && transition2.IsEpsilon) { .... } else if (!transition1.IsEpsilon && !transition2.IsEpsilon) { .... if (double.IsInfinity(transition1.Weight.Value) && double.IsInfinity(transition1.Weight.Value)) { newElementDistribution.SetToSum( 1.0, transition1.ElementDistribution, 1.0, transition2.ElementDistribution); } else { newElementDistribution.SetToSum( transition1.Weight.Value, transition1.ElementDistribution, transition2.Weight.Value, transition2.ElementDistribution); } .... }
PVS-Studio警告 ：
V3001在'&&'运算符的左侧和右侧有相同的子表达式'double.IsInfinity（transition1.Weight.Value）'。 运行时自动机.Simplification.cs 479
从代码片段中可以看到，该方法正在使用一对变量
-transition1和
transition2 。 有时使用相似名称是合理的，但是值得记住的是，在这种情况下，使用该名称在某处意外犯错的可能性会增加。
这是检查数字的无穷大（
double.IsInfinity ）时发生的情况。 由于错误，我们两次检查了相同变量的值
-transition1.Weight.Value 。 第二个子表达式中的检查值应为变量
transition2.Weight.Value 。
另一个类似的可疑代码。
internal MethodBase ToMethodInternal(IMethodReference imr) { .... bf |= BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance; .... }
PVS-Studio警告 ：
V3001在'|'的左侧和右侧有相同的子表达式'BindingFlags.Public' 操作员。 编译器CodeBuilder.cs 194
形成
bf变量的值时，将
BindingFlags.Public枚举元素使用两次。 要么此代码包含额外的标记操作，要么代替第二次使用
BindingFlags.Public，应该使用不同的枚举值。
顺便说一下，在源代码中，此代码写在一行上。 在我看来，如果将其格式化为表格样式（如此处所示），则更容易发现问题。
让我们继续前进。 我介绍了该方法的全部内容，并再次建议您自己发现错误（或者可能是错误）。
private void ForEachPrefix(IExpression expr, Action<IExpression> action) {
找到了吗 我们正在检查！
PVS-Studio警告 ：
- V3003检测到使用'if（A）{...} else if（A）{...}'模式。 存在逻辑错误的可能性。 检查行：1719、1727。编译器CodeRecognizer.cs 1719
- V3003检测到使用'if（A）{...} else if（A）{...}'模式。 存在逻辑错误的可能性。 检查行：1721，1729。编译器CodeRecognizer.cs 1721
稍微简化代码以使问题更加明显。
private void ForEachPrefix(IExpression expr, Action<IExpression> action) { if (....) .... else if (expr is IUnaryExpression) ForEachPrefix(((IUnaryExpression)expr).Expression, action); else if (expr is IAddressReferenceExpression) ForEachPrefix(((IAddressReferenceExpression)expr).Expression, action); .... else if (expr is IUnaryExpression) ForEachPrefix(((IUnaryExpression)expr).Expression, action); else if (expr is IAddressReferenceExpression) ForEachPrefix(((IAddressReferenceExpression)expr).Expression, action) .... }
条件表达式，
然后复制多个
if语句的分支。 也许这段代码是使用copy-paste方法编写的，这就是出现问题的原因。 现在事实证明，重复的
then分支将永远不会执行，因为：
- 如果条件表达式为真，则执行相应对中的第一个if语句的主体；
- 如果条件表达式在第一种情况下为false，则在第二种情况下为false。
由于那么分支包含相同的动作，因此现在看起来像是令人困惑的冗余代码。 这可能是另一种问题-应该执行其他检查，而不是重复检查。
我们继续。
public int Compare(Pair<int, int> x, Pair<int, int> y) { if (x.First < y.First) { if (x.Second >= y.Second) {
PVS-Studio警告 ：
- V3004'then '语句等效于'else'语句。 运行时RegexpTreeBuilder.cs 1080
- V3004'then '语句等效于'else'语句。 运行时RegexpTreeBuilder.cs 1093
该代码看起来非常可疑，因为它包含两个条件语句，它们具有相同的
then和
else分支主体。 在这两种情况下，可能值得返回不同的值。 或者，如果这是一种预期的行为，则删除多余的条件语句将很有用。
有有趣的周期。 下面的例子：
private static Set<StochasticityPattern> IntersectPatterns(IEnumerable<StochasticityPattern> patterns) { Set<StochasticityPattern> result = new Set<StochasticityPattern>(); result.AddRange(patterns); bool changed; do { int count = result.Count; AddIntersections(result); changed = (result.Count != count); break; } while (changed); return result; }
PVS-Studio警告 ：
V3020循环内无条件的“中断”。 编译器DefaultFactorManager.cs 474
由于无条件的
break语句，仅执行了循环的一次迭代，并且甚至没有使用
更改的控制变量。 通常，该代码看起来很奇怪和可疑。
在另一个类中发现了相同的方法（完全相同的副本）。 相应的分析仪警告：
V3020循环内无条件的“中断”。 Visualizers.Windows FactorManagerView.cs 350
顺便说一句，一种方法在循环中遇到了无条件的
continue语句（分析器使用相同的诊断程序发现了该方法），但是上面有一条注释，确认这是一个特殊的临时解决方案：
我记得在无条件
中断声明附近没有这样的评论。
让我们继续前进。
internal static DependencyInformation GetDependencyInfo(....) { .... IExpression resultIndex = null; .... if (resultIndex != null) { if (parameter.IsDefined( typeof(SkipIfMatchingIndexIsUniformAttribute), false)) { if (resultIndex == null) throw new InferCompilerException( parameter.Name + " has SkipIfMatchingIndexIsUniformAttribute but " + StringUtil.MethodNameToString(method) + " has no resultIndex parameter"); .... } .... } .... }
PVS-Studio 警告 ：
V3022表达式'resultIndex == null'始终为false。 编译器FactorManager.cs 382
立即，我注意到在声明和上面的验证之间，
resultIndex变量的值可以更改。 但是，在
两次检查之间，
resultIndex！= Null和
resultIndex == null，该值无法更改。 因此，表达式
resultIndex == null的结果将始终为
false ，这意味着永远不会引发异常。
希望您有兴趣自己发现错误，没有我的建议，找到问题，但是为了以防万一，我建议您再做一次。 方法代码很小，我将完整介绍它。
public static Tuple<int, string> ComputeMovieGenre(int offset, string feature) { string[] genres = feature.Split('|'); if (genres.Length < 1 && genres.Length > 3) { throw new ArgumentException(string.Format( "Movies should have between 1 and 3 genres; given {0}.", genres.Length)); } double value = 1.0 / genres.Length; var result = new StringBuilder( string.Format( "{0}:{1}", offset + MovieGenreBuckets[genres[0]], value)); for (int i = 1; i < genres.Length; ++i) { result.Append( string.Format( "|{0}:{1}", offset + MovieGenreBuckets[genres[i].Trim()], value)); } return new Tuple<int, string>(MovieGenreBucketCount, result.ToString()); }
让我们看看这里发生了什么。 输入字符串由字符“ |”解析。 如果数组的长度不符合预期，则必须引发异常。
等待第二个流派 ...
Length <1 && genres.Length> 3吗？ 由于没有数字立即落入表达式
[[int.MinValue..1）和
（3..int.MaxValue]所需的值的范围内，所以表达式的结果将始终为
false 。 因此，此检查不能防止任何事情，并且不会引发预期的异常。
这正是分析仪所警告的：
V3022表达式'genres.Length <1 && genres.Length> 3'始终为假。 可能是“ ||” 这里应该使用运算符。 评估器Features.cs 242
遇到可疑裂变术。
public static void CreateTrueThetaAndPhi(....) { .... double expectedRepeatOfTopicInDoc = averageDocLength / numUniqueTopicsPerDoc; .... int cnt = Poisson.Sample(expectedRepeatOfTopicInDoc); .... }
PVS-Studio警告 ：
V3041该表达式从'int'类型隐式转换为'double'类型。 考虑使用显式类型转换以避免丢失小数部分。 例如：double A =（double）（X）/ Y;。 LDA Utilities.cs 74
这是可疑的：执行了整数除法（变量
averageDocLength和
numUniqueTopicsPerDoc的类型为
int ），并将结果写入
double类型的变量。 问题是：这是专门完成的，还是仍然暗示着实数除法？ 如果
ExpectedRepeatOfTopicInDoc变量的类型为
int ，这将清除可能的问题。
在其他地方，例如，使用
Poisson.Sample方法（其参数是可疑变量
ExpectedRepeatOfTopicInDoc） ，如下所述。
int numUniqueWordsPerTopic = Poisson.Sample((double)averageWordsPerTopic);
averageWordsPerTopic的类型为
int ，已在使用位置转换为
double 。
这是另一个使用地点：
double expectedRepeatOfWordInTopic = ((double)numDocs) * averageDocLength / numUniqueWordsPerTopic; .... int cnt = Poisson.Sample(expectedRepeatOfWordInTopic);
请注意，变量的名称与原始示例中的名称相同，仅使用实数除法来初始化
ExpectedRepeatOfWordInTopic （由于
numDocs显式转换为
double ）。
通常，值得一提的是分析仪发出警告的起始位置。
但是，对于是否值得编辑以及如何编辑的思考，让代码的作者（他们更好地了解），但是让我们走得更远。 到下一个可疑师。
public static NonconjugateGaussian BAverageLogarithm(....) { .... double v_opt = 2 / 3 * (Math.Log(mx * mz / Ex2 / 2) - m); if (v_opt != v) { .... } .... }
PVS-Studio警告 ：
V3041该表达式从'int'类型隐式转换为'double'类型。 考虑使用显式类型转换以避免丢失小数部分。 例如：double A =（double）（X）/ Y;。 运行时ProductExp.cs 137
分析仪再次检测到整数除法的可疑运算，如下
2和
3是整数数字文字，并且表达式2/3的结果将为
0 。 结果，整个表达式采用以下形式：
double v_opt = 0 * expr;
同意，有点奇怪。 我几次返回到此警告，试图找到某种捕获，而不是尝试将其添加到文章中。 该方法充满了数学和各种公式（坦率地说，我并不想分解），但您永远不知道会发生什么。 此外，我尝试对本文中写出的警告尽可能地表示怀疑，并且只有在对警告进行了更好的研究之后，我才对它们进行描述。
但是后来我想到了-为什么我需要因子
0 ，写为
2/3 ？ 所以这个地方还是值得一看的。
public static void WriteAttribute(TextWriter writer, string name, object defaultValue, object value, Func<object, string> converter = null) { if ( defaultValue == null && value == null || value.Equals(defaultValue)) { return; } string stringValue = converter == null ? value.ToString() : converter(value); writer.Write($"{name}=\"{stringValue}\" "); }
PVS-Studio警告 ：
V3080可能取消空引用。 考虑检查“价值”。 编译器WriteHelpers.cs 78
根据条件对分析器进行相当合理的断言。 空引用的解引用可以在表达式
value中发生。如果
value == null，则
等于 （defaultValue） 。 由于此表达式是||运算符的右操作数，因此要计算它，左操作数必须为
false ，为此，至少一个
defaultValue \
value变量不为
null就足够了。 结果，如果
defaultValue！= Null ，并且
value == null ：
- defaultValue == null- > false ;
- defaultValue == null && value == null- > false ; （未进行价值检查）
- value.Equals（defaultValue） -> NullReferenceException ，因为value为null 。
让我们看一个类似的情况：
public FeatureParameterDistribution( GaussianMatrix traitFeatureWeightDistribution, GaussianArray biasFeatureWeightDistribution) { Debug.Assert( (traitFeatureWeightDistribution == null && biasFeatureWeightDistribution == null) || traitFeatureWeightDistribution.All( w => w != null && w.Count == biasFeatureWeightDistribution.Count), "The provided distributions should be valid and consistent in the number of features."); .... }
PVS-Studio警告 ：
V3080可能取消空引用。 考虑检查“ traitFeatureWeightDistribution”。 推荐FeatureParameterDistribution.cs 65
我们排除了多余的部分，仅保留了用于计算布尔值的逻辑，因此更容易找出：
(traitFeatureWeightDistribution == null && biasFeatureWeightDistribution == null) || traitFeatureWeightDistribution.All( w => w != null && w.Count == biasFeatureWeightDistribution.Count)
同样，||的右操作数 仅当左计算的结果为
false时才进行计算。 左操作数可以为
false ，包括
traitFeatureWeightDistribution == null和
biasFeatureWeightDistribution！= Null时 。 然后，将计算||运算符的右操作数，并调用
traitFeatureWeightDistribution.All将
引发 ArgumentNullException 。
另一段有趣的代码：
public static double GetQuantile(double probability, double[] quantiles) { .... int n = quantiles.Length; if (quantiles == null) throw new ArgumentNullException(nameof(quantiles)); if (n == 0) throw new ArgumentException("quantiles array is empty", nameof(quantiles)); .... }
PVS-Studio警告 ：
V3095在验证是否为null之前，已使用“分位数”对象。 检查行：91、92。Runtime OuterQuantiles.cs 91
请注意，
首先访问了
quantiles.Length属性，然后检查了
分位数是否为
空 。 结果，如果
分位数== null ，则该方法将引发异常，只是有一点点错误，而在不需要它的地方会抛出异常。 显然，他们弄乱了地方的线条。
如果您自己已经成功地解决了早期错误的检测问题，建议您冲泡一杯咖啡并尝试重复进行此操作，以下面的方法查找错误。 为了使它更有趣，我引用了该方法的全部代码。
（
链接到完整尺寸 ）
好吧，好吧，那是个玩笑（或者你成功了吗？！）。 让我们简化一下任务：
if (sample.Precision < 0) { precisionIsBetween = true; lowerBound = -1.0 / v; upperBound = -mean.Precision; } else if (sample.Precision < -mean.Precision) { precisionIsBetween = true; lowerBound = 0; upperBound = -mean.Precision; } else {
变得更好了吗？ 分析仪对此代码发出以下警告：
V3008 “ lowerBound”变量已连续两次分配值。 也许这是一个错误。 检查线：324，323。运行时GaussianOp.cs 324
实际上，在最后一个
else分支中，
lowerBound变量的值连续两次被分配。 显然（根据上面的代码判断），变量
upperBound必须包含在其中一个分配中。
我们会进一步。
private void WriteAucMatrix(....) { .... for (int c = 0; c < classLabelCount; c++) { int labelWidth = labels[c].Length; columnWidths[c + 1] = labelWidth > MaxLabelWidth ? MaxLabelWidth : labelWidth; for (int r = 0; r < classLabelCount; r++) { int countWidth = MaxValueWidth; if (countWidth > columnWidths[c + 1]) { columnWidths[c + 1] = countWidth; } } .... }
PVS-Studio警告 ：
V3081在嵌套循环内未使用“ r”计数器。 考虑检查“ c”计数器的用法。 命令行ClassifierEvaluationModule.cs 459
请注意，在此循环的主体中未使用内部循环的计数器-r-。 因此，事实证明，在内循环的所有迭代期间，对相同元素执行相同的操作-因为索引还使用外循环的计数器（
c ），而不使用内循环的计数器（
r ）。
让我们看看还有什么有趣的地方。
public RegexpFormattingSettings( bool putOptionalInSquareBrackets, bool showAnyElementAsQuestionMark, bool ignoreElementDistributionDetails, int truncationLength, bool escapeCharacters, bool useLazyQuantifier) { this.PutOptionalInSquareBrackets = putOptionalInSquareBrackets; this.ShowAnyElementAsQuestionMark = showAnyElementAsQuestionMark; this.IgnoreElementDistributionDetails = ignoreElementDistributionDetails; this.TruncationLength = truncationLength; this.EscapeCharacters = escapeCharacters; }
PVS-Studio警告 ：
V3117未使用构造函数参数'useLazyQuantifier'。 运行时RegexpFormattingSettings.cs 38
构造函数不使用一个参数
useLazyQuantifier 。 对于在类
UseLazyQuantifier中定义了具有相应名称和类型的属性这一事实的背景下，这尤其可疑。 显然，他们忘记了通过相应的参数对其进行初始化。
遇到了几个潜在的危险事件处理程序。 下面是其中之一的示例：
public class RecommenderRun { .... public event EventHandler Started; .... public void Execute() {
PVS-Studio警告 ：
V3083对事件'Started'的不安全调用，可能会发生NullReferenceException。 请考虑在调用事件之前将事件分配给局部变量。 评估程序RecommenderRun.cs 115
事实是，在检查
null不等式和调用处理程序之间，可以取消预订事件，并且如果在检查
null和处理程序调用之间该事件尚无订阅者，则将
抛出NullReferenceException 。 例如，要避免此类问题，您可以将指向委托链的链接保存在局部变量中，或使用“？”运算符。 调用处理程序。
除了上面的代码片段外，还有35个这样的地方。
顺便说一下，还满足了
V3024的 785条警告。 当使用运算符'！='或'=='比较实数时，会发出警告
V3024 。 我不会在这里详细说明为什么这种比较并不总是正确的-有关此问题的更多信息写在文档中，还有指向
StackOverflow的链接（就是这样）。
考虑到经常满足公式和计算的要求，这些警告也很重要，尽管它们被带到了第3级（因为它们在所有项目中都相去甚远）。
如果您确定这些警告无关紧要，则只需
单击一下即可将其删除，从而减少了分析仪操作的总数。
结论
不知何故，我很长一段时间都没有写关于检查项目的文章了，再次触摸这个过程是很愉快的。 希望您也从本文中学到了一些新的或有用的知识，或者至少有兴趣地阅读了一下。
我希望开发人员尽早纠正问题区域，并提醒您犯错误是正常现象，但我们是人。 为此，需要使用诸如静态分析仪之类的其他工具来查找某人错过的东西，对吗？ 无论如何-祝项目进展顺利，并感谢您的工作！
请记住，静态分析仪的最大好处是可以
正常使用 。
祝一切顺利！
如果您想与讲英语的读者分享这篇文章，请使用以下链接：Sergey Vasiliev。
Infer.NET代码中存在什么错误？