质量对我们很重要。 我们听说了PVS-Studio。 所有这些导致了检查Docotic.Pdf并找出其他可以改进的需求。
Docotic.Pdf是用于处理PDF文件的通用库。 它是用C＃编写的，除了.NET运行时以外，没有不安全的代码，没有外部依赖关系。 它可以在.NET 4+和.NET Standard 2+下工作。
该库已经开发了10年多，它有11万行代码，没有考虑测试，示例和其他内容。 对于静态分析，我们经常使用代码分析和StyleCop。 数以千计的自动化测试可保护我们免受回归影响。 来自不同国家和不同行业的客户对图书馆的质量表示信任。
PVS-Studio将检测到哪些问题？
安装和第一印象
我从PVS-Studio网站下载了试用版。 安装程序的体积小巧，令人惊喜。 使用默认设置安装：分析引擎，单独的PVS-Studio环境，集成到Visual Studio 2017中。
安装后，没有任何启动，并且带有相同图标的两个快捷方式被添加到“开始”菜单中：独立和PVS-Studio。 一会儿，我想到了从哪里开始。 启动独立版本，界面令人不愉快。 弯曲地支持为Windows设置的200％缩放比例。 文本的一部分太小，文本的一部分不适合为其提供的空间。 名称，独角兽和动作列表将针对任何窗口大小进行裁剪。 即使全屏显示。
好吧，好的，我决定打开我的项目文件。 突然，“文件”菜单没有找到这样的机会。 在那里，我只被提供打开个人文件。 谢谢，我想，我想尝试其他选择。 推出PVS-Studio-他们向我展示了一个带有模糊文字的窗口。 再次达到200％的规模。 文字报告：
在“三冠王”中寻找
我，在Visual Studio中寻找PVS-Studio菜单。 好的，打开Studio。
打开的解决方案。 实际上，有一个PVS-Studio菜单，它具有检查“当前项目”的功能。 他将我需要的项目列为最新项目，并启动了一项检查。 Studio中会弹出一个窗口，其中包含分析结果。 随着扫描的进行，背景中出现了一个窗口，但我没有立即找到它。 最初，感觉到支票没有开始或立即结束。
第一次检查结果
分析仪在大约9分钟30秒内检查了所有1253个项目文件。 到检查结束时，文件计数器的更改速度没有开始时的快。 扫描持续时间对扫描的文件数量可能存在某种非线性依赖性。
结果窗口中出现有关81高，109中和175低警告的信息。 如果计算频率，则将得到0.06高警告/文件，0.09中警告/文件和0.14低警告/文件。 或
每千行代码0.74高警告，每千行代码0.99中警告，每千行代码1.59低警告。
本文中的此处
表示 ，在CruiseControl.NET中，该程序具有25.6万行代码，分析器发现了15个高警告，151个中警告和32个低警告。
事实证明，就Docotic.Pdf而言，每个组中均发出了更多警告。
发现了什么？
我决定在此阶段忽略低警告。
我按“代码”列对警告进行了排序，结果发现频率的绝对记录保存者为
V3022 “表达式始终为true / false”和
V3063 “条件表达式的一部分在被评估时始终为true / false”。 我认为它们只是一件事。 总共这两个警告给出了190个案例中的92个，相对频率= 48％。
分为高和中的逻辑尚不完全清楚。 我期待
V3072 “包含IDisposable成员的'A'类本身并不实现IDisposable”和
V3073 “并非所有IDisposable成员都得到适当处置。 例如，在“高级”组中布置“ A”类时，请调用“布置”。 但这当然是味道。
惊讶于
V3095 “在针对null进行验证之前使用了该对象。 检查行：N1，N2被标记为两次高，一次标记为中。 虫子？
信任但要验证
现在该检查警告的合理性了。 是否发现任何实际错误？ 是否有任何不正确的警告？
我将发现的警告分为以下几组。
重要警告
他们的修正提高了稳定性，解决了内存泄漏等问题。 真正的错误/缺陷。
其中有16个已发布，占所有警告的8％。
我会举一些例子。
V3019 “可能在使用'as'关键字进行类型转换后，将不正确的变量与null进行比较。 检查变量“颜色”，“索引”»
public override bool IsCompatible(ColorImpl color) { IndexedColorImpl indexed = color as IndexedColorImpl; if (color == null) return false; return indexed.ColorSpace.Equals(this); }
如您所见，将变量color与null进行比较，而不是对其进行索引。 这是不正确的，并可能导致NRE。
V3080 “可能取消空引用。 考虑检查“ cstr_index.tile_index”»
一个小片段来说明：
if (cstr_index.tile_index == null) { if (cstr_index.tile_index[0].tp_index == null) {
显然，第一个条件隐含！=空。 在当前形式下，代码将在每次调用时抛出NRE。
V3083 “事件'OnProgress'的不安全调用，可能会发生NullReferenceException。 请考虑在调用事件之前将事件分配给局部变量。”
public void Updated() { if (OnProgress != null) OnProgress(this, new EventArgs()); }
警告有助于解决潜在的异常。 为什么会出现？ Stackoverflow有一个
很好的解释 。
V3106 “可能索引超出范围。 索引“ 0”指向“ v”界限之外»
var result = new List<FontStringPair>(); for (int i = 0; i < text.Length; ++i) { var v = new List<FontStringPair>(); createPairs(text[i].ToString(CultureInfo.InvariantCulture)); result.Add(v[0]); }
错误是将忽略createPairs的结果，而是访问一个空列表。 显然，最初createPairs接受列表作为参数，但是在更改方法的过程中发生了错误。
V3117 '构造函数参数'validateType'未使用
针对与此类似的代码发出了警告
public SomeClass(IDocument document, bool validateType = true) : base(document, true) { m_provider = document; }
警告本身似乎并不重要。 但是这个问题比乍看之下要严重得多。 在添加可选的validateType参数的过程中，他们忘记了将调用固定到基类的构造函数。
V3127？找到了两个类似的代码片段。 也许，这是一个错字，应该使用“范围”变量而不是“域”“
private void fillTransferFunction(PdfStreamImpl function) {
如果代码的部分稍有不同，则可能不会发出警告。 但是在这种情况下，使用复制粘贴时检测到错误。
理论/形式警告
它们要么是正确的，但是它们的更正不能解决任何特定的错误，并且不影响代码的可读性。 或者他们指出可能存在错误但不存在的地方。 例如，有意改变参数的顺序。 对于此类警告，您无需在程序中进行任何更改。
在这些警告中，有57个已发出，占所有警告的30％。 我将举一些值得关注的案例。
V3013 “奇怪的是，“ BeginText”功能的主体完全等同于“ EndText”功能的主体（166，第171行）”
public override void BeginText() { m_state.ResetTextParameters(); } public override void EndText() { m_state.ResetTextParameters(); }
两种身体功能实际上是相同的。 但这是对的。 如果一行的功能主体重合真的真的很奇怪吗？
V3106？可能的负索引值。 “ c1”索引的值可能达到-1“
freq[256] = 1;
我同意，我给出了一个不太清楚的算法。 但是，在我看来，在这种情况下，分析仪是徒劳的。
V3107 “在化合物分配的左侧和右侧相同的表达式'neighsum'”
该警告是由一个很普通的代码引起的：
neighsum += neighsum;
是的，可以通过乘法重写。 但是没有错。
V3109 “ l_cblk.data_current_size”子表达式存在于运算符的两侧。 该表达式不正确或可以简化。
if ((l_cblk.data_current_size + l_seg.newlen) < l_cblk.data_current_size) {
代码中的注释阐明了意图。 再次虚惊一场。
正当警告
他们的更正对代码的可读性产生了积极的影响。 也就是说，它减少了不必要的条件，检查等。 对代码工作方式的影响并不明显。
在这些警告中，有103个被发布，占所有警告的54％。
V3008 “ l_mct_deco_data”变量被连续两次分配值。 也许这是一个错误
if (m_nb_mct_records == m_nb_max_mct_records) { ResizeMctRecords(); l_mct_deco_data = (OPJ_INT32)m_nb_mct_records; } l_mct_deco_data = (OPJ_INT32)m_nb_mct_records;
权限分析器：如果不需要，在内部分配。
V3009 “奇怪的是，该方法总是返回一个相同的值”
private static bool opj_dwt_decode_tile(opj_tcd_tilecomp_t tilec, OPJ_UINT32 numres) { if (numres == 1U) return true;
根据分析仪的建议，该方法已更改，仅返回任何内容。
V3022 “表达式'！Add'始终为真”
private void addToFields(PdfDictionaryImpl controlDict, bool add) {
的确，第二个if毫无意义。 条件将始终为真。
V3029 “
彼此并排放置的'if'语句的条件表达式相同”
if (stroke) extGState.OpacityStroke = opacity; if (stroke) state.AddReal(Field.CA, opacity);
目前尚不清楚这种代码是如何产生的。 但是现在我们修复了它。
V3031？过度检查可以简化。 “ ||” 运算符被相反的表达式包围“
这是一场噩梦：
if (!(cp.m_enc.m_tp_on != 0 && ((!opj_codec_t.OPJ_IS_CINEMA(cp.rsiz) && (t2_mode == J2K_T2_MODE.FINAL_PASS)) || opj_codec_t.OPJ_IS_CINEMA(cp.rsiz)))) {
更改后，情况变得更好了
if (!(cp.m_enc.m_tp_on != 0 && (opj_codec_t.OPJ_IS_CINEMA(cp.rsiz) || t2_mode == J2K_T2_MODE.FINAL_PASS))) {
V3063 “条件表达式的一部分如果被求值，则始终为true：x！= Null”
V3022 “表达式'x！= Null'始终为真”
在这里，我包含了警告，检查null没有意义。 这样做是否正确是一个有争议的问题。 我在下面更详细地描述了问题的实质。
无根据的警告
误报 由于特定测试的实施中的错误或某种分析仪缺陷。
其中，发布了14个，占所有警告的7％。
V3081 “未在嵌套循环内使用'i'计数器。 考虑检查“ j”计数器的使用情况“
发出此警告的代码的稍微简化的版本：
for (uint i = 0; i < initialGlyphsCount - 1; ++i) { for (int j = 0; j < initialGlyphsCount - i - 1; ++j) {
显然，我是在嵌套循环中使用的。
V3125 “对象已针对null进行验证后使用”
发出警告的代码：
private static int Compare_SecondGreater(cmapEncodingRecord er1, cmapEncodingRecord er2) { if (er1 == er2) return 0; if (er1 != null && er2 == null) return 1; if (er1 == null && er2 != null) return -1; return er1.CompareTo(er2); }
调用CompareTo（）时er1不能为null。
发出此警告的另一个代码：
private static void realloc(ref int[][] table, int newSize) { int[][] newTable = new int[newSize][]; int existingSize = 0; if (table != null) existingSize = table.Length; for (int i = 0; i < existingSize; i++) newTable[i] = table[i]; for (int i = existingSize; i < newSize; i++) newTable[i] = new int[4]; table = newTable; }
table在循环中不能为null。
V3134 “移位[32..255]位大于'UInt32'类型的表达式'（uint）1'的大小”
发出此警告的一段代码：
byte bitPos = (byte)(numBits & 0x1F); uint mask = (uint)1 << bitPos;
可以看出bitPos的值可以在[0..31]范围内，但分析仪认为它的值可以在[0..31]范围内，这是不正确的。
我不会给出其他类似情况，因为它们是等效的。
一些检查的其他想法
在我看来，不希望警告“ x！= Null”在x是调用某种方法的结果的情况下始终为真。 一个例子：
private X GetX(int y) { if (y > 0) return new X(...); if (y == 0) return new X(...); throw new ArgumentOutOfRangeException(nameof(x)); } private Method() {
是的，从形式上讲，分析器是正确的：x永远不会为null，因为GetX要么返回完整实例，要么抛出异常。 但是，代码是否可以将检查的清除效果提高为null？ 如果GetX以后更改怎么办？ Method是否必须知道GetX实现？
在团队内部，意见分歧。 有人建议当前方法有一个约定，根据该约定不应返回null。 而且，每次调用都编写“面向未来”的冗余代码是没有意义的。 并且如果合同更改，则必须更新调用代码。
为支持此观点，做出了以下判断：检查null就像将每个调用包装在try / catch中，以防万一该方法在将来开始引发异常。
结果，根据
YAGNI原则，他们决定不保留支票并将其删除。 所有警告都从理论/正式转向合理。
我很高兴在评论中阅读您的意见。
结论
静态分析是一件好事。 PVS-Studio可让您发现真正的错误。
是的，有不合理/错误的警告。 但是，PVS-Studio仍然在已经使用代码分析的项目中发现了真正的错误。 我们的产品相当受测试覆盖，它是客户测试的一种或另一种方法，但是
机器人做得更好，静态分析仍然是有益的。
最后是一些统计数据。
前3个不合理的警告
V3081嵌套循环中不使用“ X”计数器。 考虑检查“ Y”计数器的使用情况“
1人中有1人发现不合理
V3125 “将对象验证为空后，将使用该对象。 检查行：N1，N2“
10人中有9人被宣布为毫无根据
V3134 “ N位移位大于类型的大小”
5人中有4人被发现是没有根据的
前3个重要警告
V3083 “事件的不安全调用，可能是NullReferenceException。 考虑在调用事件之前将事件分配给本地变量“
5人中有5人被认为很重要。
V3020 “循环内无条件的'中断/继续/返回/跳转'”
V3080 “可能的空取消引用”
2人中有2人被认为很重要。
V3019 “在使用'as'关键字进行类型转换后，有可能将不正确的变量与null比较”
V3127？找到了两个类似的代码片段。 也许，这是一个错字，应该使用“ X”变量而不是“ Y”
1中的1被认为很重要。