# Шпион под прикрытием: проверяем исходный код ILSpy с помощью PVS\-Studio В блоге компании PVS\-Studio можно найти далеко не одну статью с результатами проверок исходного кода различных компиляторов\. С другой стороны, немного обделённым вниманием PVS\-Studio кажется другой класс программ \- класс декомпиляторов\. Дабы сделать мир более гармоничным, был проверен исходный C\# код программы ILSpy, как раз относящейся к лагерю декомпиляторов\. Давайте же посмотрим, что интересного смог найти PVS\-Studio\. ![0794_ILSpy_ru/image1.png](https://import.viva64.com/docx/blog/0794_ILSpy_ru/image1.png) ## Введение Наверное, каждому программисту хоть раз в жизни приходилось использовать декомпилятор\. Цели у каждого из нас могли быть разными: узнать имплементацию какого\-то метода, подтвердить или опровергнуть свои подозрения насчёт ошибки в используемой библиотеке, или просто под влиянием любопытства посмотреть близкий к исходному код интересующей нас программы\. В мире \.NET'а при упоминании слова декомпилятор обычно в голову приходит или dotPeek или ILSpy\. Про \.NET Reflector сейчас вспоминают меньше\. Припоминаю, как, когда в первый раз узнал про подобный класс утилит и декомпилировал чужую библиотеку, в голове пробежала мысль о шпионаже\. И такие мысли были не только у меня одного \- не спроста же декомпилятор ILSpy получил своё название\. Итак, в данной статье я описал интересные и подозрительные с моей точки зрения места, обнаруженные с помощью PVS\-Studio в исходном коде проекта ILSpy\. Захотелось посмотреть, так сказать, что скрывает наш шпион и, в случае необходимости, "прикрыть" его статическим анализатором\. Откровенно говоря, статья про ILSpy получилась несколько случайно\. Среди пользователей нашего анализатора достаточно много студий, занимающихся игровой разработкой\. Это одна из причин того, почему мы в компании стараемся сделать наш инструмент как можно более полезным и удобным для разработчиков игр, в том числе использующих движки Unity и Unreal Engine\. Я знаком с клиентами, которые используют PVS\-Studio совместно с Unreal Engine, а вот про Unity разработчиков, практикующих использование нашего анализатора, слышу значительно реже\. В связи с этим хочется популяризировать анализатор среди Unity сообщества\. Одним из способов популяризации могла бы стать статья о проверке open\-source игры, разработанной при помощи данного движка\. Вот только здесь у меня возникла проблема \- я не смог найти такую игру \(может вы, читатели, сможете любезно [предоставить идеи для таких проверок](https://github.com/viva64/pvs-studio-check-list)?\)\. Обстоятельства при поиске игры с открытым исходным кодом складывались немного странно, и на [одном сайте](https://awesomeopensource.com/projects/unity) в списке самых популярных проектов для Unity оказался ILSpy \(для меня остаётся загадкой, как и почему он попал в этот список\)\. К слову, ILSpy входит в пул проектов, на которых мы регулярно тестируем наш C\# анализатор внутри компании\. Странно, что статьи про этот проект у нас до сих пор не было\. Ну что ж, раз не удалось найти Unity проект для анализа, так давайте проверим попавшийся на глаза ILSpy\. Вот что написано в описание проекта на [GitHub'е](https://github.com/icsharpcode/ILSpy): ILSpy is the open\-source \.NET assembly browser and decompiler\. Я не нашёл информацию о том, что при разработке проекта используется какой\-нибудь статический анализатор\. Наверное, так даже интереснее \- PVS\-Studio будет первым\. Перейдём непосредственно к результатам анализа\. ## Замена без замены [V3038](https://pvs-studio.ru/ru/docs/warnings/v3038/) The '"'"' argument was passed to 'Replace' method several times\. It is possible that other argument should be passed instead\. ICSharpCode\.Decompiler ReflectionDisassembler\.cs 772 ```cpp private static void WriteSimpleValue(ITextOutput output, object value, string typeName) { switch (typeName) { case "string": output.Write( "'" + DisassemblerHelpers .EscapeString(value.ToString()) .Replace("'", "\'") // <= + "'"); break; case "type": .... } .... } ``` Кажется, что автор хотел осуществить замену всех вхождений символа одинарной кавычки на строку, состоящую из двух символов: символа обратной косой черты и символа одинарной кавычки\. Из\-за невнимательности вышла осечка \- символ "'" меняется на самого себя, что является бессмысленной операцией\. Между присвоением строке значения "'" и "\\'" разницы нет \- в любом случае строка будет проинициализирована символом одинарной кавычки\. Если мы хотим, чтобы в строку попало значение "\\'", то backslash нужно экранировать или воспользоваться символом '@': "\\\\'" или @"\\'"\. То есть вызов метода _Replace_ следует переписать следующим образом: ```cpp Replace("'", @"\'") ``` ## Правда и только правда **Предупреждение 1** [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'negatedOp \=\= BinaryOperatorType\.Any' is always true\. ICSharpCode\.Decompiler CSharpUtil\.cs 79 ```cpp static Expression InvertConditionInternal(Expression condition) { var bOp = (BinaryOperatorExpression)condition; if ( (bOp.Operator == BinaryOperatorType.ConditionalAnd) || (bOp.Operator == BinaryOperatorType.ConditionalOr)) { .... } else if ( (bOp.Operator == BinaryOperatorType.Equality) || (bOp.Operator == BinaryOperatorType.InEquality) || (bOp.Operator == BinaryOperatorType.GreaterThan) || (bOp.Operator == BinaryOperatorType.GreaterThanOrEqual) || (bOp.Operator == BinaryOperatorType.LessThan) || (bOp.Operator == BinaryOperatorType.LessThanOrEqual)) { .... } else { var negatedOp = NegateRelationalOperator(bOp.Operator); if (negatedOp == BinaryOperatorType.Any) // <= return new UnaryOperatorExpression(....); bOp = (BinaryOperatorExpression)bOp.Clone(); bOp.Operator = negatedOp; return bOp; } } ``` Анализатор предупреждает, что значение переменной _negatedOp_ всегда равно значению _Any_ из перечисления _BinaryOperatorType_\. Чтобы убедиться в этом, давайте посмотрим на код метода _NegateRelationalOperator_, из которого данная переменная и получает своё значение\. ```cpp public static BinaryOperatorType NegateRelationalOperator(BinaryOperatorType op) { switch (op) { case BinaryOperatorType.GreaterThan: return BinaryOperatorType.LessThanOrEqual; case BinaryOperatorType.GreaterThanOrEqual: return BinaryOperatorType.LessThan; case BinaryOperatorType.Equality: return BinaryOperatorType.InEquality; case BinaryOperatorType.InEquality: return BinaryOperatorType.Equality; case BinaryOperatorType.LessThan: return BinaryOperatorType.GreaterThanOrEqual; case BinaryOperatorType.LessThanOrEqual: return BinaryOperatorType.GreaterThan; case BinaryOperatorType.ConditionalOr: return BinaryOperatorType.ConditionalAnd; case BinaryOperatorType.ConditionalAnd: return BinaryOperatorType.ConditionalOr; } return BinaryOperatorType.Any; } ``` Если к моменту вызова метода _NegateRelationalOperator_ переменная _bOp\.Operator_ имела значение, несоответствующее ни одной метке _case_, то из метода вернётся значение _BinaryOperatorType\.Any_\. Видно, что вызов метода _NegateRelationalOperator_ происходит только в том случае, если условия в вышестоящем операторе _if_ и операторе _if else_ были вычислены как _false_\. А если быть совсем внимательным, то становится заметно, что условия в операторах _if_ и _if_ _else_ покрывают все метки _case_ из метода _NegateRelationalOperator_\. Следовательно, к моменту вызова метода _NegateRelationalOperator_ переменная _bOp\.Operator_ не подходит ни под одну метку _case_, и этот метод в данном случае всегда вернёт значение _BinaryOperatorType\.Any_\. Вот и получается, что _negatedOp \=\= BinaryOperatorType\.Any_ всегда оценивается как _true_, и на следующей строке происходит возврат значения из метода\. Вдобавок получаем недостижимый код: ```cpp bOp = (BinaryOperatorExpression)bOp.Clone(); bOp.Operator = negatedOp; return bOp; ``` К слову, анализатор любезно выдал предупреждение и на это: [V3142](https://pvs-studio.ru/ru/docs/warnings/v3142/) Unreachable code detected\. It is possible that an error is present\. ICSharpCode\.Decompiler CSharpUtil\.cs 81 **Предупреждение 2** [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'pt \!\= null' is always true\. ICSharpCode\.Decompiler FunctionPointerType\.cs 168 ```cpp public override IType VisitChildren(TypeVisitor visitor) { .... IType[] pt = (r != ReturnType) ? new IType[ParameterTypes.Length] : null; .... if (pt == null) return this; else return new FunctionPointerType( module, CallingConvention, CustomCallingConventions, r, ReturnIsRefReadOnly, pt != null ? pt.ToImmutableArray() : ParameterTypes, // <= ParameterReferenceKinds); } ``` Здесь всё очевидно \- ветка _else_ выполняется только в том случае, если переменная _pt_ не равна _null_\. Зачем тогда нужно писать тернарный оператор с проверкой переменной _pt_ на неравенство _null,_ мне непонятно\. Возможно, в прошлом не было ветвления _if_ _else_ и первого оператора _return_\. Тогда подобная проверка имела бы смысл, ну а сейчас всё же стоит убрать лишний тернарный оператор: ```cpp public override IType VisitChildren(TypeVisitor visitor) { .... IType[] pt = (r != ReturnType) ? new IType[ParameterTypes.Length] : null; .... if (pt == null) return this; else return new FunctionPointerType( module, CallingConvention, CustomCallingConventions, r, ReturnIsRefReadOnly, pt.ToImmutableArray(), ParameterReferenceKinds); } ``` **Предупреждение 3** [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'settings\.LoadInMemory' is always true\. ICSharpCode\.Decompiler CSharpDecompiler\.cs 394 ```cpp static PEFile LoadPEFile(string fileName, DecompilerSettings settings) { settings.LoadInMemory = true; return new PEFile( fileName, new FileStream(fileName, FileMode.Open, FileAccess.Read), streamOptions: settings.LoadInMemory ? // <= PEStreamOptions.PrefetchEntireImage : PEStreamOptions.Default, metadataOptions: settings.ApplyWindowsRuntimeProjections ? MetadataReaderOptions.ApplyWindowsRuntimeProjections : MetadataReaderOptions.None ); } ``` Аналогично предыдущему срабатыванию анализатора получаем совершенно ненужный тернарный оператор\. Свойству _settings\.LoadInMemory_, проверяемому в тернарном оператора, выше присваивается значение _true_, которое не меняется вплоть до самого тернарного оператора\. Для полноты картины приведу код геттера и сеттера самого свойства: ```cpp public bool LoadInMemory { get { return loadInMemory; } set { if (loadInMemory != value) { loadInMemory = value; OnPropertyChanged(); } } } ``` Думаю, переписанный метод без тернарного оператора приводить не стоит \- тут всё достаточно бесхитростно\. **Предупреждение 4** [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'ta' is always not null\. The operator '??' is excessive\. ICSharpCode\.Decompiler ParameterizedType\.cs 354 ```cpp public IType VisitChildren(TypeVisitor visitor) { .... if (ta == null) return this; else return new ParameterizedType(g, ta ?? typeArguments); // <= } ``` Тут уже натыкаемся на ненужный _null_ _coalescing_ оператор\. При попадании в ветку _else_ переменная _ta_ всегда имеет значение неравное _null_\. Как следствие, использование оператора ?? тут лишнее\. Всего было найдено 31 предупреждение с номером [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/)\. ## Ты здесь лишний **Предупреждение 1** [V3025](https://pvs-studio.ru/ru/docs/warnings/v3025/) Incorrect format\. A different number of format items is expected while calling 'Format' function\. Arguments not used: End\. ICSharpCode\.Decompiler Interval\.cs 269 ```cpp public override string ToString() { if (End == long.MinValue) { if (Start == long.MinValue) return string.Format("[long.MinValue..long.MaxValue]", End); // <= else return string.Format("[{0}..long.MaxValue]", Start); } else if (Start == long.MinValue) { return string.Format("[long.MinValue..{0})", End); } else { return string.Format("[{0}..{1})", Start, End); } } ``` При вызове самого первого метода _string\.Format_ строка форматирования не соответствует передаваемым в метод фактическим аргументам\. Значение переменной _End_, передаваемое в качестве аргумента, не будет подставлено в строку форматирования, так как в ней отсутствует элемент форматирования \{0\}\. Исходя из логики данного метода это всё же не ошибка, первый оператор _return_ вернёт именно ту строку, которую и задумывали авторы кода\. Это, разумеется, не отменяет того факта, что присутствует бесполезный вызов метода _string\.Format_ с неиспользуемым аргументом\. Это хорошо бы исправить, дабы не вводить в заблуждения человека, который будет читать этот метод\. **Предупреждение 2** [V3025](https://pvs-studio.ru/ru/docs/warnings/v3025/) Incorrect format\. A different number of format items is expected while calling 'AppendFormat' function\. Arguments not used: angle\. ILSpy\.BamlDecompiler XamlPathDeserializer\.cs 177 ```cpp public static string Deserialize(BinaryReader reader) { .... var sb = new StringBuilder(); .... sb.AppendFormat(CultureInfo.InvariantCulture, "A{0} {2:R} {2} {3} {4}", size, angle, largeArc ? '1' : '0', sweepDirection ? '1' : '0', pt1); .... } ``` В данном случае за бортом оказалась переменная _angle_\. Несмотря на то, что её передали в метод _AppendFormat_, из\-за того, что в строке форматирования отсутствует элемент форматирования \{1\} и дважды использован \{2\}, это переменная остаётся неиспользованной\. Скорее всего, авторы хотели написать строку формата следующим образом: _"A\{0\} \{1:R\} \{2\} \{3\} \{4\}"_\. ## Двойные стандарты **Предупреждение 1** [V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/) The 'roslynProject' object was used before it was verified against null\. Check lines: 96, 97\. ILSpy\.AddIn OpenILSpyCommand\.cs 96 ```cpp protected Dictionary GetReferences(....) { .... var roslynProject = owner.Workspace .CurrentSolution .GetProject(projectReference.ProjectId); var project = FindProject(owner.DTE.Solution .Projects.OfType(), roslynProject.FilePath); // <= if (roslynProject != null && project != null) // <= .... } ``` Сначала мы обращаемся к свойству _FilePath_ объекта _roslynProject_ без какого\-либо опасения, что в переменной _roslynProject_ может быть записан _null_, а буквально строкой ниже мы выполняем проверку равенства этой переменной на _null_\. Такой код выглядит небезопасно и чреват возникновением исключения типа _NullReferenceException_\. Для исправления подобной ситуации стоит обращаться к свойству _FilePath_, используя null\-условный оператор, а в методе _FindProject_ предусмотреть возможность получения потенциального _null_ в качестве последнего параметра\. **Предупреждение 2** [V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/) The 'listBox' object was used before it was verified against null\. Check lines: 46, 52\. ILSpy FlagsFilterControl\.xaml\.cs 46 ```cpp public override void OnApplyTemplate() { base.OnApplyTemplate(); listBox = Template.FindName("ListBox", this) as ListBox; listBox.ItemsSource = FlagGroup.GetFlags(....); // <= var filter = Filter; if (filter == null || filter.Mask == -1) { listBox?.SelectAll(); // <= } } ``` Ситуация аналогична предыдущему примеру\. Сначала обращаемся к свойству _ItemsSource_ без какой\-либо проверки, что в переменной _listBox_ может быть записан _null_, а несколькими строками ниже уже видим использование null\-условного оператора с переменной _listBox_\. Причём переменная _listBox_ между двумя этими обращениями к полю и методу не получала нового значения\. Всего анализатор нашёл 10 предупреждений с номером [V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/)\. Привожу список данных предупреждений: * V3095 The 'pV' object was used before it was verified against null\. Check lines: 761, 765\. ICSharpCode\.Decompiler TypeInference\.cs 761 * V3095 The 'pU' object was used before it was verified against null\. Check lines: 882, 886\. ICSharpCode\.Decompiler TypeInference\.cs 882 * V3095 The 'finalStore' object was used before it was verified against null\. Check lines: 261, 262\. ICSharpCode\.Decompiler TransformArrayInitializers\.cs 261 * V3095 The 'definitionDeclaringType' object was used before it was verified against null\. Check lines: 93, 104\. ICSharpCode\.Decompiler SpecializedMember\.cs 93 * V3095 The 'TypeNamespace' object was used before it was verified against null\. Check lines: 84, 88\. ILSpy\.BamlDecompiler XamlType\.cs 84 * V3095 The 'property\.Getter' object was used before it was verified against null\. Check lines: 1676, 1684\. ICSharpCode\.Decompiler CSharpDecompiler\.cs 1676 * V3095 The 'ev\.AddAccessor' object was used before it was verified against null\. Check lines: 1709, 1717\. ICSharpCode\.Decompiler CSharpDecompiler\.cs 1709 * V3095 The 'targetType' object was used before it was verified against null\. Check lines: 1614, 1657\. ICSharpCode\.Decompiler CallBuilder\.cs 1614 Кстати, если вы хотите проверить свой собственный проект с помощью PVS\-Studio или перепроверить тот же ILSpy, чтобы более детально изучить срабатывания, то можете собственноручно [попробовать анализатор](https://pvs-studio.ru/ru/pvs-studio/download/)\. Перейдя на сайт PVS\-Studio, вы сможете как скачать сам анализатор, так и получить триальную лицензию\. ## Всё идёт к одному **Предупреждение 1** [V3139](https://pvs-studio.ru/ru/docs/warnings/v3139/) Two or more case\-branches perform the same actions\. ILSpy Images\.cs 251 ```cpp protected override ImageSource GetBaseImage(MemberIcon icon) { ImageSource baseImage; switch (icon) { case MemberIcon.Field: baseImage = Images.Field; break; case MemberIcon.FieldReadOnly: baseImage = Images.FieldReadOnly; break; case MemberIcon.Literal: baseImage = Images.Literal; // <= break; case MemberIcon.EnumValue: baseImage = Images.Literal; // <= break; case MemberIcon.Property: baseImage = Images.Property; break; case MemberIcon.Indexer: baseImage = Images.Indexer; break; case MemberIcon.Method: baseImage = Images.Method; break; case MemberIcon.Constructor: baseImage = Images.Constructor; break; case MemberIcon.VirtualMethod: baseImage = Images.VirtualMethod; break; case MemberIcon.Operator: baseImage = Images.Operator; break; case MemberIcon.ExtensionMethod: baseImage = Images.ExtensionMethod; break; case MemberIcon.PInvokeMethod: baseImage = Images.PInvokeMethod; break; case MemberIcon.Event: baseImage = Images.Event; break; default: throw new ArgumentOutOfRangeException(nameof(icon), $"MemberIcon.{icon} is not supported!"); } return baseImage; } ``` На мой взгляд, это явная ошибка\. В случае, если переменная _icon_ равна _MemberIcon\.EnumValue_, то переменная _baseImage_ в ветке _case_ должна получать значение _Images\.EnumValue_\. Это хороший пример ошибки, который легко замечает статический анализатор, и легко пропускает человек при беглом обзоре кода\. **Предупреждение 2** [V3139](https://pvs-studio.ru/ru/docs/warnings/v3139/) Two or more case\-branches perform the same actions\. ICSharpCode\.Decompiler CSharpConversions\.cs 829 ```cpp bool ImplicitConstantExpressionConversion(ResolveResult rr, IType toType) { .... switch (toTypeCode) { case TypeCode.SByte: return val >= SByte.MinValue && val <= SByte.MaxValue; case TypeCode.Byte: return val >= Byte.MinValue && val <= Byte.MaxValue; case TypeCode.Int16: return val >= Int16.MinValue && val <= Int16.MaxValue; case TypeCode.UInt16: return val >= UInt16.MinValue && val <= UInt16.MaxValue; case TypeCode.UInt32: return val >= 0; // <= case TypeCode.UInt64: return val >= 0; // <= } .... } ``` Утверждать, что анализатор нашёл здесь явную ошибку я не берусь, но предупреждение однозначно имеет смысл\. Если метки _case_ для значения _TypeCode\.UInt32_ и _TypeCode\.UInt64_ выполняют один и тот же набор действий, почему бы не написать код более компактно: ```cpp bool ImplicitConstantExpressionConversion(ResolveResult rr, IType toType) { switch (toTypeCode) { .... case TypeCode.UInt32: case TypeCode.UInt64: return val >= 0; } .... } ``` Анализатор выдал ещё 2 предупреждения с номером [V3139](https://pvs-studio.ru/ru/docs/warnings/v3139/): * V3139 Two or more case\-branches perform the same actions\. ICSharpCode\.Decompiler EscapeInvalidIdentifiers\.cs 85 * V3139 Two or more case\-branches perform the same actions\. ICSharpCode\.Decompiler TransformExpressionTrees\.cs 370 ## Безопасность превыше всего [V3083](https://pvs-studio.ru/ru/docs/warnings/v3083/) Unsafe invocation of event, NullReferenceException is possible\. Consider assigning event to a local variable before invoking it\. ILSpy MainWindow\.xaml\.cs 787class ResXResourceWriter : IDisposable ```cpp void assemblyList_Assemblies_CollectionChanged(....) { .... if (CurrentAssemblyListChanged != null) CurrentAssemblyListChanged(this, e); // <= } ``` Подобный способ вызова событий является достаточно распространённым, но то, что мы встречаем данный паттерн во многих проектах, не является оправданием к его применению\. Разумеется, это не критичная ошибка, но, как и говорит текст предупреждения анализатора, \- этот вызов не является безопасным, и не исключено возникновение исключения типа _NullReferenceException_\. Если между проверкой _CurrentAssemblyListChanged_ на _null_ и вызовом самого события от него отписались все обработчики \(например, в другом потоке исполнения\), то произойдёт выброс исключения _NullReferenceException_\. Лучше сразу писать безопасный код, например, следующим образом: ```cpp void assemblyList_Assemblies_CollectionChanged(....) { .... CurrentAssemblyListChanged?.Invoke(this, e); } ``` PVS\-Studio обнаружил ещё 8 подобных случаев, и все их можно исправить аналогично представленному выше способу\. ## Уверенная неуверенность [V3146](https://pvs-studio.ru/ru/docs/warnings/v3146/) Possible null dereference\. The 'FirstOrDefault' can return default null value\. ILSpy\.BamlDecompiler BamlResourceEntryNode\.cs 76 ```cpp bool LoadBaml(AvalonEditTextOutput output, CancellationToken cancellationToken) { var asm = this.Ancestors().OfType() .FirstOrDefault().LoadedAssembly; // <= .... return true; } ``` Из коллекции, возвращаемой методом _OfType_, посредством вызова метода _FirstOrDefault_ хотят получить первый встретившийся элемент типа _AssemblyTreeNode_\. Все мы знаем, что, если в коллекции не будет встречено такого элемента, который бы удовлетворял предикату поиска, или, если сама коллекция оказалось пустой, то метод _FirstOrDefault_ вернёт значение по умолчанию \- _null_ в нашем случае\. Дальнейшее обращение к свойству _LoadedAssembly_ по нулевой ссылке приведёт к возникновению исключения типа _NullReferenceException_\. Соответственно, чтобы избежать подобной ситуации следует использовать null\-условный оператор: ```cpp bool LoadBaml(AvalonEditTextOutput output, CancellationToken cancellationToken) { var asm = this.Ancestors().OfType() .FirstOrDefault()?.LoadedAssembly; // <= .... return true; } ``` Можно предположить, что разработчик уверен в том, что в данном конкретном месте метод _FirstOrDefault_ никогда не вернёт _null_\. Если подобная ситуация имеет место быть, то тогда лучше воспользоваться методом _First_ вместо _FirstOrDefault_, так как он подчёркивает нашу уверенность в том, что мы всегда достанем нужный элемент из коллекции\. Тем более, если элемент не будет найден в коллекции, то мы получим исключение типа _InvalidOperationException_ \(а не _NullReferenceException_ как в случае с использованием _FirstOrDefault_ с последующим обращением к свойству по нулевой ссылке\) с понятным сообщением: "Sequence contains no elements"\. ## Небезопасное сканирование [V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) The 'm' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. ILSpy MethodVirtualUsedByAnalyzer\.cs 137 ```cpp static bool ScanMethodBody(IMethod analyzedMethod, IMethod method, MethodBodyBlock methodBody) { .... var mainModule = (MetadataModule)method.ParentModule; .... switch (member.Kind) { case HandleKind.MethodDefinition: case HandleKind.MethodSpecification: case HandleKind.MemberReference: var m = (mainModule.ResolveEntity(member, genericContext) as IMember) ?.MemberDefinition; if ( m.MetadataToken == analyzedMethod.MetadataToken // <= && m.ParentModule.PEFile == analyzedMethod.ParentModule.PEFile) // <= { return true; } break; } .... } ``` При инициализации переменной _m_ использовался null\-условный оператор, следовательно, предполагается, что _m_ потенциально может иметь значение _null_\. Интересно, что сразу на следующей строке мы уже без использования null\-условного оператора обращаемся к свойствам переменной _m_, что чревато возникновением исключения типа _NullReferenceException_\. Как и в некоторых других примерах из данной статьи исправляем ситуацию с помощью использования null\-условного оператора: ```cpp static bool ScanMethodBody(IMethod analyzedMethod, IMethod method, MethodBodyBlock methodBody) { .... var mainModule = (MetadataModule)method.ParentModule; .... switch (member.Kind) { case HandleKind.MethodDefinition: case HandleKind.MethodSpecification: case HandleKind.MemberReference: var m = (mainModule.ResolveEntity(member, genericContext) as IMember) ?.MemberDefinition; if ( m?.MetadataToken == analyzedMethod.MetadataToken && m?.ParentModule.PEFile == analyzedMethod.ParentModule.PEFile) { return true; } break; } .... } ``` ## Старые знакомые [V3070](https://pvs-studio.ru/ru/docs/warnings/v3070/) Uninitialized variable 'schema' is used when initializing the 'ResourceSchema' variable\. ICSharpCode\.Decompiler ResXResourceWriter\.cs 63 ```cpp class ResXResourceWriter : IDisposable { .... public static readonly string ResourceSchema = schema; .... static string schema = ....; .... } ``` Изначально я не планировал выписывать это предупреждения\. Дело в том, что абсолютно идентичная ошибка уже была найдена пять лет назад в результате проверки проекта [Mono](https://pvs-studio.ru/ru/blog/posts/csharp/0431/), но после небольшого обсуждения с коллегой мы пришли к выводу, что упомянуть её в статье всё\-таки стоит\. Как и описано в статье про проверку Mono, на момент инициализации статического поля _ResourceSchema_ другим статическим полем _schema_, поле _schema_ ещё само не инициализировано и имеет значение по умолчанию \- _null_\. Файл ResXResourceWriter\.cs, в котором была найдена ошибка, был любезно позаимствован с сохранением авторских прав из проекта Mono\. Файл был расширен некоторым уникальным для проекта ILSpy функционалом\. Вот так баги из одно проекта расползаются по сети и кочуют из одного проекта в другой\. Кстати, в первоисточнике ошибку до сих пор не поправили\. ## Заключение Подытоживая разбор результатов анализа, можно сказать, что были найдены не только фрагменты исходного кода, которые просто желательно переписать в целях рефакторинга, но и реальные ошибки, где авторы кода явно ожидают другого результата \(например, тот же вызов метода _Replace_ с одинаковыми аргументами\)\. Регулярное использование статического анализа позволяет быстрее находить и исправлять подобные подозрительные места\. Всегда проще и дешевле править баг на стадии написания/тестирования кода, нежели чем в продакшене, когда о баге вам уже сообщает пользователь, а не статический анализатор\. Спасибо за прочтение\. ## Примечание для тех, кто хочет самостоятельно проверить ILSpy При анализе проекта ILSpy мы обнаружили несколько проблем, связанных с самим анализатором \- да, случается и такое\. Мы исправили проблемы, но правки не вошли в релиз версии 7\.11, начиная со следующей версии они будут доступны\. Так же в связи с тем, что проект ILSpy собирается немного нестандартно, потребовались некоторые дополнительные настройки анализатора\. Так что, если вы хотите проверить ILSpy самостоятельно \- [напишите нам](https://pvs-studio.ru/ru/about-feedback/)\. Мы выдадим бету анализатора и расскажем, как настроить проверку\.