Новогодний релиз PVS-Studio 6.00: проверяем Roslyn

    PVS-Studio 6.00, C,C++,C#
    Настало долгожданное событие. Мы выпустили релизную версию статического анализатора кода PVS-Studio 6.00, поддерживающего проверку C#-проектов. Теперь осуществляется проверка кода, написанного на следующих языках: C, C++, C++/CLI, C++/CX, C#. К выпуску шестой версии анализатора мы приурочили проверку открытого проекта Roslyn. Именно благодаря Roslyn в анализаторе PVS-Studio появилась поддержка C#, и мы очень благодарны компании Microsoft за реализацию и развитие этого проекта.

    PVS-Studio 6.00


    PVS-Studio — статический анализатор кода, ориентированный на простоту использования и поиск ошибок на этапе написания кода.

    Мы постоянно добавляем диагностические правила для поиска новых разновидностей ошибок в программах на языке C/C++. Например, недавно мы добавили поиск членов класса, которые не инициализируется в конструкторе, что было весьма непростой задачей. Однако, усовершенствование диагностик не является поводом смены старшей версии. И мы ждали, когда добавим в анализатор что-то по-настоящему новое. И вот такой момент настал. Мы представляем миру шестую версию анализатора, поддерживающую язык C#.

    Триальная версия PVS-Studio 6.00 доступна по ссылке:

    http://www.viva64.com/ru/pvs-studio-download/

    В шестой версии анализатора мы отказались от поддержки старых версий Visual Studio. Теперь не поддерживается VS2005 и VS2008. Если ваша команда по-прежнему использует эти версии Visual Studio, то мы предлагаем продолжить использовать предыдущую версию 5.31 или её апгрейды, если такие появятся.

    Демонстрационная версия имеет следующее ограничение. Можно выполнить 50 переходов по коду. После этого анализатор предложит человеку отправить информацию о себе. Если он согласится, то ему будет предоставлено ещё 50 переходов.

    Ограничение демонстрационной версии может показаться суровым. Однако ему есть обоснование, и мы пришли к нему после многих экспериментов.

    Малое количество «кликов» поможет быстрее начать общение в почте. Если кто-то захочет посмотреть на другие сообщения, то мы готовы выдать ему регистрационный ключ, скажем, на 1 неделю. Ему достаточно написать нам письмо. Как правило, в процессе переписки мы помогаем быстрее сориентироваться человеку, как он может быстро и просто получить пользу от использования анализатора.

    Roslyn


    Программисты не восприимчивы к рекламе. Их невозможно увлечь словами «миссия», «безупречно» и «фокус на инновациях». Программист не читает рекламные анонсы и знает, как отключить баннеры с помощью Adblock Plus.

    Единственное, чем их можно увлечь, это показать, как тот или иной инструмент может быть им полезен. Мы избрали именно такой путь и показываем, какую пользу может приносить инструмент статического анализа, проверяя открытые проекты.

    PVS-Studio умеет находить ошибки в таких известных проектах как CoreCLR, LibreOffice, Linux Kernel, Qt, Unreal Engine 4, Chromium и так далее. На данный момент наша команда проверила 230 открытых проектов и обнаружила в них 9355 ошибок. Да, да 9355 — это количество ошибок, а не количество диагностических сообщений. С наиболее интересными проверками можно познакомиться в соответствующих статьях.

    Теперь очередь дошла и до проверки C# проектов. Не удивительно, что одним из первых проверенных проектов стал Roslyn. В конце концов, благодаря именно этому проекту и появилась возможность поддержать в PVS-Studio анализ C# кода.

    Хочу выразить благодарность компании Microsoft за то, что она создала этот открытый проект. Он окажет большое влияние на развитие C#-инфраструктуры. Да он уже оказывает влияние! Например, на базу Rolsyn переносятся такие известные проекты как ReSharper или CodeRush.

    Теперь немного о самом проекте Roslyn.

    .NET Compiler Platform, более известная под кодовым названием Roslyn, — это набор компиляторов с открытым кодом и API для анализа исходного кода для языков C# и Visual Basic .NET. Платформа разрабатывается компанией Microsoft.

    Данная платформа включает в себя самодостаточные компиляторы для C# и VB.NET. Они доступны не только в виде традиционных приложений командной строки, но и в качестве нативных API, к которым можно обращаться из .NET-кода. Roslyn открывает доступ к модулям синтаксического (лексического) анализа кода, семантического анализа, динамической компиляции в CIL-байткод и генерации сборки. Предоставляемые Roslyn API можно разделить на три типа: функциональные API (feature API), API рабочего пространства (workspace API) и API компиляторов (compiler API). Функциональные API облегчают процесс рефакторинга и отладки. API рабочего пространства позволяют разработчикам плагинов реализовывать определенные механизмы, специфичные для IDE наподобие Visual Studio — например, поиск соответствий между переменными и значениями или форматирование кода. API компиляторов обеспечивают возможности для еще более продвинутого анализа исходного кода, предоставляя информацию о прямых вызовах модулей анализа синтаксического дерева и анализа потока управления на этапе связывания идентификаторов со значениями.

    Дополнительные ссылки:
    1. GitHub. Roslyn.
    2. Wikipedia. .NET Compiler Platform («Roslyn»)
    3. .NET Compiler Platform («Roslyn») Overview.
    4. MSDN. Forum. Microsoft «Roslyn» CTP.
    5. MSDN. Taking a tour of Roslyn.
    6. Learn Roslyn Now.
    7. Miguel de Icaza. Mono and Roslyn.

    Найденные ошибки


    Анализатор PVS-Studio нашел мало ошибок в Roslyn. Это естественно для столь известного проекта, разрабатываемого компанией Microsoft с её отлаженными системами контроля качества. Найти хоть что-то в коде Roslyn уже является большой победой, и мы гордимся, что можем это сделать.

    Многие из приведенных далее ошибок относятся к тестам или обработчикам ошибок. Это естественно. Ошибки в часто используемых фрагментах кода исправляются благодаря тестированию и отзывам пользователям. Но для нас важен сам факт, что PVS-Studio может находить ошибки. Это означает, что многие ошибки, которые были исправлены с большими трудозатратами, могли бы исправляться сразу при регулярном использовании PVS-Studio.

    Еще раз, другими словами. Ценность анализатора не в разовых запусках, а в его регулярном использовании. Статический анализ кода можно рассматривать как расширенный вариант предупреждений компилятора. Глупо включать предупреждения компилятора раз в год. На предупреждения нужно смотреть сразу после их возникновения. Это экономит время разработки на поиск глупых багов с помощью отладки. Со статическим анализатором всё то же самое.

    Давайте посмотрим, что интересного удалось обнаружить с помощью PVS-Studio:

    Ошибка N1 в тестах. Copy-Paste.
    public void IndexerMemberRace()
    {
      ....
      for (int i = 0; i < 20; i++)
      {
        ....
        if (i % 2 == 0)
        {
          thread1.Start();
          thread2.Start();
        }
        else
        {
          thread1.Start();
          thread2.Start();
        }
        ....
      }
      ....
    }

    Предупреждение PVS-Studio: V3004 The 'then' statement is equivalent to the 'else' statement. GetSemanticInfoTests.cs 2269

    Вот один из примеров ошибки в тестах, которая может жить в проекте годами, так как никому не мешает. Просто тест проверяет не всё, что планировалось. В начале всегда стартует поток 1, а потом поток 2. Скорее всего, планировалось написать тест так:
    if (i % 2 == 0)
    {
      thread1.Start();
      thread2.Start();
    }
    else
    {
      // Поменяли порядок запуска потоков
      thread2.Start();
      thread1.Start();
    }

    Ошибка N2 в тестах. Опечатка.
    public DiagnosticAsyncToken(
      AsynchronousOperationListener listener,
      string name,
      object tag,
      string filePath,
      int lineNumber)
      : base(listener)
    {
      Name = Name;
      Tag = tag;
      FilePath = filePath;
      LineNumber = lineNumber;
      StackTrace = PortableShim.StackTrace.GetString();
    }

    Предупреждение PVS-Studio: V3005 The 'Name' variable is assigned to itself. AsynchronousOperationListener.DiagnosticAsyncToken.cs 32

    На взгляд ошибку здесь найти весьма непросто. Подвело неудачное именование переменных. Имена свойств класса отличаются от аргументов функции только первой заглавной буквой. Как результат — легко допустить опечатку, что и произошло: Name = Name.

    Ошибка N3, N4. Copy-Paste.
    private Task<SyntaxToken>
    GetNewTokenWithRemovedOrToggledPragmaAsync(....)
    {
      var result = isStartToken
        ? GetNewTokenWithPragmaUnsuppress(
            token, indexOfTriviaToRemoveOrToggle, _diagnostic, Fixer,
            isStartToken, toggle)
        : GetNewTokenWithPragmaUnsuppress(
            token, indexOfTriviaToRemoveOrToggle, _diagnostic, Fixer,
            isStartToken, toggle);
            
      return Task.FromResult(result);
    }

    Предупреждение PVS-Studio: V3012 The '?:' operator, regardless of its conditional expression, always returns one and the same value. AbstractSuppressionCodeFixProvider.RemoveSuppressionCodeAction_Pragma.cs 177

    Вне зависимости от значения 'isStartToken' функция GetNewTokenWithPragmaUnsuppress() вызывается с одним и тем же набором фактических аргументов. Скорее всего, вызов функции был продублирован с помощью Copy-Paste, а потом что-то забыли изменить в скопированном коде.

    Вот ещё один похожий случай:
    private void DisplayDiagnostics(....)
    {
      ....
      _console.Out.WriteLine(
        string.Format((notShown == 1) ?
          ScriptingResources.PlusAdditionalError :
          ScriptingResources.PlusAdditionalError,
          notShown));
      ....
    }

    Предупреждение PVS-Studio: V3012 The '?:' operator, regardless of its conditional expression, always returns one and the same value: ScriptingResources.PlusAdditionalError. CommandLineRunner.cs 428

    Ошибки N5, N6. Невнимательность.

    У меня ещё мало статистики по типовым ошибкам, которые допускают C# программисты. Но помимо стандартных опечаток явно начинает лидировать следующий паттерн ошибки: часто после приведения ссылки с помощью оператора 'as' проверяют не получившеюся ссылку, а исходную. После чего используют непроверенную ссылку. Синтетический код:
    var A = B as T;
    if (B) A.Foo();

    А вот как эта ошибка выглядит на практике:
    public override bool Equals(object obj)
    {
      var d = obj as DiagnosticDescription;
    
      if (obj == null)
        return false;
        
      if (!_code.Equals(d._code))
        return false;
      ....
    }

    Предупреждение PVS-Studio: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'obj', 'd'. DiagnosticDescription.cs 201

    Следующий пример более многословен, но на самом деле всё тоже самое:
    protected override bool AreEqual(object other)
    {
      var otherResourceString = other as LocalizableResourceString;
      return
        other != null &&
        _nameOfLocalizableResource == 
          otherResourceString._nameOfLocalizableResource &&
        _resourceManager == otherResourceString._resourceManager &&
        _resourceSource == otherResourceString._resourceSource &&
        ....
    }

    Предупреждение PVS-Studio: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'other', 'otherResourceString'. LocalizableResourceString.cs 121

    Ошибка N7. Двойное обнаружение.

    О наличии ошибки в коде иногда свидетельствуют сразу 2, а то и 3 предупреждения анализатора. Сейчас мы как раз рассмотрим такой случай.
    private bool HasMatchingEndTag(
      XmlElementStartTagSyntax parentStartTag)
    {
      if (parentStartTag == null)
      {
        return false;
      }
    
      var parentElement = parentStartTag.Parent as XmlElementSyntax;
      if (parentStartTag == null)
      {
        return false;
      }
      var endTag = parentElement.EndTag;
      ....
    }

    Предупреждения PVS-Studio:
    • V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'parentStartTag', 'parentElement'. XmlTagCompletionCommandHandler.cs 123
    • V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless XmlTagCompletionCommandHandler.cs 117

    В начале функции проверяется, что аргумент 'parentStartTag' не является нулевой ссылкой. Если она нулевая, то происходит выход из функции.

    Затем хотели проверить, что ссылка на самом деле указывает на класс типа 'XmlElementSyntax'. Но в этот момент в код вкралась опечатка. Вместо того, чтобы проверить 'parentElement', повторно проверяется 'parentStartTag'.

    Анализатор замечает сразу две аномалии. Первая аномалия: нет смысла повторно проверять значение 'parentStartTag', так как если это нулевая ссылка, то мы уже покинули функцию. Вторая аномалия: анализатор подозревает, что проверяется не та переменная после оператора 'as'.

    Корректный вариант кода:
    if (parentStartTag == null)
    {
      return false;
    }
    var parentElement = parentStartTag.Parent as XmlElementSyntax;
    if (parentElement == null)
    {
      return false;
    }

    Ошибки N8, N9. Copy-Paste.

    Прошу прощения за длинный фрагмент кода, но не хотелось заменять его на синтетический пример.
    internal static bool ReportConflictWithParameter(....)
    {
      ....
      if (newSymbolKind == SymbolKind.Parameter ||
          newSymbolKind == SymbolKind.Local)
      {
        diagnostics.Add(ErrorCode.ERR_LocalSameNameAsTypeParam,
                        newLocation, name);
        return true;
      }
      if (newSymbolKind == SymbolKind.TypeParameter)
      {
        return false;
      }
      if (newSymbolKind == SymbolKind.Parameter ||
          newSymbolKind == SymbolKind.Local)
      {
        diagnostics.Add(ErrorCode.ERR_LocalSameNameAsTypeParam,
                        newLocation, name);
        return true;
      }
      ....
    }

    Предупреждение PVS-Studio: V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless InMethodBinder.cs 264

    В приведенном в статье фрагменте кода совпадает первый и третий оператор 'if'. Видимо блок был скопирован, а затем в нем что-то забыли изменить. Хотя возможно, повторный 'if' просто лишний и его следует удалить.

    Есть ещё один подобный участок кода, но я не буду мучить читателя длинным примером. Просто приведу диагностическое предупреждение:

    V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless WithLambdaParametersBinder.cs 131

    Ошибка N10. Неверное условие.
    public enum TypeCode
    {
      ....
      Object = 1,
      ....
      DateTime = 16,
      ....
    }
    
    static object GetHostObjectValue(Type lmrType, object rawValue)
    {
      var typeCode = Metadata.Type.GetTypeCode(lmrType);
      return (lmrType.IsPointer || lmrType.IsEnum ||
              typeCode != TypeCode.DateTime ||
              typeCode != TypeCode.Object)
                ? rawValue : null;
    }

    Предупреждение PVS-Studio: V3022 Expression 'lmrType.IsPointer || lmrType.IsEnum || typeCode != TypeCode.DateTime || typeCode != TypeCode.Object' is always true. DkmClrValue.cs 136

    Выражение достаточно сложное, поэтому я выделю главную суть:
    (typeCode != 1 || typeCode != 16)

    Это выражение всегда является истинным, независимо от значения переменной 'typeCode'.

    Ошибка N11. Избыточное условие.
    public enum EventCommand
    {
      Disable = -3,
      Enable = -2,
      SendManifest = -1,
      Update = 0
    }
    
    protected override void OnEventCommand(
      EventCommandEventArgs command)
    {
      base.OnEventCommand(command);
    
      if (command.Command == EventCommand.SendManifest ||
          command.Command != EventCommand.Disable ||
           FunctionDefinitionRequested(command))
      ....
    }

    Предупреждение PVS-Studio: V3023 Consider inspecting this expression. The expression is excessive or contains a misprint. RoslynEventSource.cs 79

    Вновь выделю суть:
    if (A == -1 || A != -3)

    Это выражение ошибочно или избыточно. Его можно сократить до:
    if (A != -3)

    Ошибка N12 при логировании.
    static CompilerServerLogger()
    {
      ....
      loggingFileName = Path.Combine(loggingFileName,
        string.Format("server.{1}.{2}.log",
                      loggingFileName,
                      GetCurrentProcessId(),
                      Environment.TickCount));
      ....
    }

    Предупреждение PVS-Studio: V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Expected: 2. Present: 3. CompilerServerLogger.cs 49

    Переменная 'loggingFileName' никак не используется при вызове функции Format(). Это подозрительно.

    Ошибка N13 в обработчике ошибок.
    private const string WriteFileExceptionMessage =
      @"{1}
      To reload the Roslyn compiler package, close Visual Studio and
      any MSBuild processes, then restart Visual Studio.";
      
    private void WriteMSBuildFiles(....)
    {
      ....
      catch (Exception e)
      {
        VsShellUtilities.ShowMessageBox(
          this,
          string.Format(WriteFileExceptionMessage, e.Message),
          null,
          OLEMSGICON.OLEMSGICON_WARNING,
          OLEMSGBUTTON.OLEMSGBUTTON_OK,
          OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST);
      }
    }

    Предупреждение PVS-Studio: V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Expected: 2. Present: 1. CompilerPackage.cs 105

    По всей видимости при попытке показать Message Box будет сгенерировано исключение. Дело в том, что функция Format() попытается распечатать второй дополнительный аргумент. А его нет.

    Мне кажется, константная строка, задающая форматирование, должна начинаться с:
    @"{0} 

    Ошибка N14, N15 в обработчике ошибок.

    Я готов поспорить, что функция DumpAttributes() пока никак не используется. В ней есть сразу 2 ошибки, каждая из которых должна приводить к генерации исключения:
    private void DumpAttributes(Symbol s)
    {
      int i = 0;
      foreach (var sa in s.GetAttributes())
      {
        int j = 0;
        foreach (var pa in sa.CommonConstructorArguments)
        {
          Console.WriteLine("{0} {1} {2}", pa.ToString());
          j += 1;
        }
        j = 0;
        foreach (var na in sa.CommonNamedArguments)
        {
          Console.WriteLine("{0} {1} {2} = {3}",
            na.Key, na.Value.ToString());
          j += 1;
        }
        i += 1;
      }
    }

    Предупреждения PVS-Studio:
    • V3025 Incorrect format. A different number of format items is expected while calling 'WriteLine' function. Expected: 3. Present: 1. LoadingAttributes.cs 551
    • V3025 Incorrect format. A different number of format items is expected while calling 'WriteLine' function. Expected: 4. Present: 2. LoadingAttributes.cs 558

    В обоих случаях вызова функции WriteLine(), ей передается меньше фактических аргументов, чем требуется. Как результат — должны возникать исключения FormatException.

    Ошибка N16. Опасное выражение.

    Уверен, что сейчас вы посмотрите на следующий фрагмент кода и не захотите в нем разбираться. Это хорошо показывает, как полезны неутомимые анализаторы кода.
    private static bool SymbolsAreCompatibleCore(....)
    {
      ....
      var type = methodSymbol.ContainingType;
      var newType = newMethodSymbol.ContainingType;
      if ((type != null && type.IsEnumType() &&
           type.EnumUnderlyingType != null &&
           type.EnumUnderlyingType.SpecialType ==
             newType.SpecialType) ||
          (newType != null && newType.IsEnumType() &&
           newType.EnumUnderlyingType != null &&
           newType.EnumUnderlyingType.SpecialType ==
             type.SpecialType))
      {
        return true;
      }
      ....
    }

    Предупреждение PVS-Studio: V3027 The variable 'newType' was utilized in the logical expression before it was verified against null in the same logical expression. AbstractSpeculationAnalyzer.cs 383

    Чтобы пояснить, в чем опасность кода, я составлю на его основе простой синтетический пример:
    if ((A != null && A.x == B.y) || (B != null && B.q == A.w))

    Как видите, подразумевается, что A и B могут являться нулевыми ссылками. Выражение состоит из двух частей. В первой части проверяется ссылка A, но не проверена ссылка B. Во второй части проверяется ссылка B, но не проверена ссылка A.

    Возможно код и работает благодаря везению, но выглядит очень подозрительным и опасным.

    Ошибки N17, N18. Повторные присваивания.
    public static string Stringize(this Diagnostic e)
    {
      var retVal = string.Empty;
      if (e.Location.IsInSource)
      {
        retVal = e.Location.SourceSpan.ToString() + ": ";
      }
      else if (e.Location.IsInMetadata)
      {
        return "metadata: ";
      }
      else
      {
        return "no location: ";
      }
      retVal = e.Severity.ToString() + " " + e.Id + ": " +
               e.GetMessage(CultureInfo.CurrentCulture);
      return retVal;
    }

    Предупреждение PVS-Studio: V3008 The 'retVal' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 324, 313. DiagnosticExtensions.cs 324

    Обратите внимание, что в одной из веток оператора 'if' переменной 'retVal' присваиваются значение. Однако в конце функции значение переменной 'retVal' перезаписывается. Я не уверен, но возможно последнее присваивание должно быть таким:
    retVal = retVal  + e.Severity.ToString() + " " + e.Id + ": " +
             e.GetMessage(CultureInfo.CurrentCulture);

    Рассмотрим ещё один аналогичный случай:
    public int GetMethodsInDocument(
      ISymUnmanagedDocument document,
      int bufferLength, 
      out int count,
      ....)
    {
      ....
      if (bufferLength > 0)
      {
        ....
        count = actualCount;
      }
      else
      {
        count = extentsByMethod.Length;
      }
      count = 0;
      return HResult.S_OK;
    }

    Предупреждение PVS-Studio: V3008 The 'count' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 317, 314. SymReader.cs 317

    Функция возвращает значение по ссылке 'count'. В разных частях функции в 'count' записываются различные значения. Подозрительно то, что в конце функции в 'count' вдруг всегда записывается 0. Очень странно.

    Ошибка N19 в тестах. Опечатка.
    internal void VerifySemantics(....)
    {
      ....
      if (additionalOldSources != null)
      {
        oldTrees = oldTrees.Concat(
          additionalOldSources.Select(s => ParseText(s)));
      }
      
      if (additionalOldSources != null)
      {
        newTrees = newTrees.Concat(
          additionalNewSources.Select(s => ParseText(s)));
      }
      ....
    }

    Предупреждение PVS-Studio: V3029 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 223, 228. EditAndContinueTestHelpers.cs 223

    Во втором условии следовало проверять не 'additionalOldSources', а 'additionalNewSources'. Если ссылка 'additionalNewSources' вдруг окажется нулевой, то будет сгенерировано исключение при попытке вызова функции Select().

    Ошибка N20. Из спорного.

    Естественно, в статье я разобрал далеко не все предупреждения, которые выдал анализатор PVS-Studio. Есть немало явно ложных срабатываний, но ещё больше ситуаций, где у меня просто нет знаний о проекте Roslyn, чтобы судить, содержит код ошибку, или нет. Рассмотрим один из таких случаев.
    public static SyntaxTrivia Whitespace(string text)
    {
      return Syntax.InternalSyntax.SyntaxFactory.Whitespace(
               text, elastic: false);
    }
    public static SyntaxTrivia ElasticWhitespace(string text)
    {
      return Syntax.InternalSyntax.SyntaxFactory.Whitespace(
               text, elastic: false);
    }

    V3013 It is odd that the body of 'Whitespace' function is fully equivalent to the body of 'ElasticWhitespace' function (118, line 129). SyntaxFactory.cs 118

    Две функции имеют идентичные тела. Это подозрительно с точки зрения анализатора. Подозрительны эти функции и мне. Но я не знаю проект, и, возможно, код написан совершенно правильно. Поэтому я выскажу только предположение. Возможно, внутри функции ElasticWhitespace() следует использовать фактический параметр 'elastic' равный 'true'.

    Ошибка Nxx.

    Надеюсь, читатели понимают, что я не могу подробно разбираться с каждым таким случаем, как был показан выше. Я проверяю много проектов и каждый из них мне не знаком. Поэтому я описываю в статьях только наиболее очевидные ошибки. В этой статье я остановился на 20 таких случаях. Однако, думаю, PVS-Studio смог обнаружить гораздо больше дефектов. Поэтому предлагаю разработчикам Roslyn не полагаться только на эту статью, а проверить проект самостоятельно. Для этого демонстрационной версии анализатора будет недостаточно, но мы готовы предоставить на время лицензионный ключ.

    Сравнение с ReSharper


    Я написал ещё совсем немного статей про проверку C# проектов и выступил только на одной конференции. Но уже понял, что каждый раз будет задаваться вопрос: «А у вас есть сравнение с ReSharper?».

    Мне этот вопрос не нравится по двум причинам. Во-первых, это всё-таки инструменты разного типа. PVS-Studio это классический анализатор кода, ориентированный на поиск ошибок. ReSharper это Productivity Tool, предназначенный облегчить программирование и умеющий выдавать большое количество рекомендаций.

    PVS-Studio и ReSharper имеют совершенно разные подходы по многим вопросам. Смотрите, например, у PVS-Studio есть документация с подробным описанием каждой диагностики, примерами и советами по исправлению кода. В случае ReSharper есть утверждение о «1400 code inspections in C#, VB.NET, XAML, XML, ASP.NET, ASP.NET MVC, Razor, JavaScript, TypeScript, HTML, CSS, ResX». Число 1400 выглядит солидно, но на самом деле ни о чем не говорит. Возможно где-то и есть описание всех этих code inspection, но сходу я не смог его найти. Как мне сравнивать инструмент, если я даже не могу прочитать о том, какие именно ошибки обнаруживает ReSharper в C# программах.

    Во-вторых, любое сравнение, которое мы сделаем, будет полностью раскритиковано. Мы уже проходили это и зареклись впредь делать какие-либо сравнения. Например, мы тщательно подошли к сравнению PVS-Studio с Cppcheck и Visual Studio SCA. Было потрачено много времени и сил. Результаты были представлены в кратком и подробном варианте. После чего только ленивый не написал, что мы всё сделали неправильно, или что наше сравнение является нечестным благодаря специально выбранным проектам для проверки.

    Не видим смысл тратить время на сравнение. Как бы тщательно и честно мы не подошли к нему, всегда можно будет сказать, что сравнение было предвзято.

    Тем не менее должен быть ответ, лучше ли мы в чем-то, чем ReSharper. И такой ответ у меня есть.

    Вы уже используете ReSharper? Отлично! Теперь установите PVS-Studio и найдите в своём проекте ошибки!

    Заключение


    Предлагаю не откладывая скачать PVS-Studio и проверить свой проект. Видите ошибки? А ведь у вас вполне работающий код. Скорее всего найденные ошибки расположены в редко используемых участках кода. В часто используемых участках кода подобные ошибки хоть медленно и мучительно, но были исправлены. Представьте теперь, как много мог бы сэкономить сил анализатор PVS-Studio при регулярном использовании. Конечно анализатор способен найти далеко не все ошибки. Но чем тратить время на поиски опечаток и ляпов, лучше провести его с пользой. А глупые ошибки отдайте на растерзание PVS-Studio.

    P.S. Вы не делаете глупых ошибок? Ну-ну. Это только так кажется. Все делают. Посмотрите хотя-бы сюда.


    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. New Year PVS-Studio 6.00 Release: Scanning Roslyn.

    Прочитали статью и есть вопрос?
    Часто к нашим статьям задают одни и те же вопросы. Ответы на них мы собрали здесь: Ответы на вопросы читателей статей про PVS-Studio, версия 2015. Пожалуйста, ознакомьтесь со списком.
    PVS-Studio
    785,38
    Static Code Analysis for C, C++, C# and Java
    Поделиться публикацией

    Комментарии 38

      +17
      Хочу выразить благодарность компании Microsoft за то, что она создала этот открытый проект. Он окажет большое влияние на развитие C#-инфраструктуры. Да он уже оказывает влияние! Например, на базу Rolsyn переносятся такие известные проекты как ReSharper или CodeRush:
      И по вашей же ссылке текст:
      The short answer to this tremendously popular question is, no, ReSharper will not use Roslyn. There are at least two major reasons behind this.
      Вы вообще читаете то, на что ссылаетесь?
        –16
        Можно было и в личную почту написать. У меня было мало времени на подготовку статьи. :)
          +8
          + есть замечательное видео с конференции DotNext 2015

          +8
          Поразительно! Я был довольно скептически настроен, но за час нашел кучу ошибок, некоторые критичные, которые случаются в редких случаях

          Не реклама, просто огромная благодарность :)
            +2
            image
            image
            image
            image
            image
              0
              Надеемся что когда-нибудь PVS улучшит вид этого окна в темной теме Visual Studio
              +2
              image
              image
              image
              image
              image

                +2
                А решарпер почему не используете?
                  +4
                  используем, но только как помощник для написания кода. все его warnings уже давно все игнорируют.
                  Вот на том же проекте он сейчас выдал 5429 issues, понятно что никто не будет с этим разбираться
                    0
                    Лучше уж кому нибудь разобраться с ними и отключить все шумные предупреждения для проекта (там можно настройки закомитить с проектом), а самые полезные исправлять
                      0
                      Вот на том же проекте он сейчас выдал 5429 issues, понятно что никто не будет с этим разбираться

                      все его warnings уже давно все игнорируют.

                      Если вы отключили решарперовские варнинги — то где гарантия, что и предупреждения PVS не отправятся в тот же игнор?
                        +1
                        Я думаю у нас получится в PVS-Studio сделать C# анализ с очень низким уровнем ложных срабатываний.
                    0
                    Все эти identical subexpressions намекают на то, что вы не очень-то аккуратно программируете. Если switch не покрывает все кейсы, это нормально. Как вы умудрились посчитать All() по перечислению и при этом никуда не присвоить — большой вопрос.
                      0
                      почему-то перестал работать

                      Exception message:
                      There was no endpoint listening at net.pipe://localhost/33cde34b-307f-4abb-9c31-2a1dd644aa6b/PVSStudioAnalyzer that could accept the message. This is often caused by an incorrect address or SOAP action. See InnerException, if present, for more details.

                      Exception type:
                      System.ServiceModel.EndpointNotFoundException

                      Stack:
                      Server stack trace:
                      at System.ServiceModel.Channels.ConnectionUpgradeHelper.DecodeFramingFault(ClientFramingDecoder decoder, IConnection connection, Uri via, String contentType, TimeoutHelper& timeoutHelper)
                      at System.ServiceModel.Channels.ClientFramingDuplexSessionChannel.SendPreamble(IConnection connection, ArraySegment`1 preamble, TimeoutHelper& timeoutHelper)
                      at System.ServiceModel.Channels.ClientFramingDuplexSessionChannel.DuplexConnectionPoolHelper.AcceptPooledConnection(IConnection connection, TimeoutHelper& timeoutHelper)
                      at System.ServiceModel.Channels.ConnectionPoolHelper.EstablishConnection(TimeSpan timeout)
                      at System.ServiceModel.Channels.ClientFramingDuplexSessionChannel.OnOpen(TimeSpan timeout)
                      at System.ServiceModel.Channels.CommunicationObject.Open(TimeSpan timeout)
                      at System.ServiceModel.Channels.ServiceChannel.OnOpen(TimeSpan timeout)
                      at System.ServiceModel.Channels.CommunicationObject.Open(TimeSpan timeout)
                      at System.ServiceModel.Channels.ServiceChannel.CallOpenOnce.System.ServiceModel.Channels.ServiceChannel.ICallOnce.Call(ServiceChannel channel, TimeSpan timeout)
                      at System.ServiceModel.Channels.ServiceChannel.CallOnceManager.CallOnce(TimeSpan timeout, CallOnceManager cascade)
                      at System.ServiceModel.Channels.ServiceChannel.EnsureOpened(TimeSpan timeout)
                      at System.ServiceModel.Channels.ServiceChannel.Call(String action, Boolean oneway, ProxyOperationRuntime operation, Object[] ins, Object[] outs, TimeSpan timeout)
                      at System.ServiceModel.Channels.ServiceChannelProxy.InvokeService(IMethodCallMessage methodCall, ProxyOperationRuntime operation)
                      at System.ServiceModel.Channels.ServiceChannelProxy.Invoke(IMessage message)

                      Exception rethrown at [0]:
                      at System.Runtime.Remoting.Proxies.RealProxy.HandleReturnMessage(IMessage reqMsg, IMessage retMsg)
                      at System.Runtime.Remoting.Proxies.RealProxy.PrivateInvoke(MessageData& msgData, Int32 type)
                      at ProgramVerificationSystems.PVSStudio.Analyzer.Integration.IAnalyzerContract.StartAnalysis(IDictionary`2 projectToFilesMap, Boolean obfuscateResults)
                      at ProgramVerificationSystems.PVSStudio.Analyzer.Integration.AnalyzerServiceSession.StartRemoteAnalysis(IDictionary`2 projectFilesMap, String channelName) in D:\CCNetProjects\PVS-Studio Setup\WorkDir\PVS-Studio\PVS-Studio\Analyzer\Integration\AnalyzerServiceSession.cs:line 192
                      at ProgramVerificationSystems.PVSStudio.Analyzer.Integration.AnalyzerServiceSession.Execute(IDictionary`2 projectFilesMap, String platform, String configuration, String solutionName, CancellationToken cancelToken, Boolean obfuscateResults) in D:\CCNetProjects\PVS-Studio Setup\WorkDir\PVS-Studio\PVS-Studio\Analyzer\Integration\AnalyzerServiceSession.cs:line 106
                      at ProgramVerificationSystems.PVSStudio.ProcessingEngine.<>c__DisplayClass53_0.b__10() in D:\CCNetProjects\PVS-Studio Setup\WorkDir\PVS-Studio\PVS-Studio\ProcessingEngine.cs:line 799

                      PVS-Studio Version: 6.0.15162.1

                      Visual Studio Version: 14.0 Professional

                      Visual Studio LocaleID: en

                      Windows Version: Microsoft Windows NT 10.0.10240.0
                        0
                        Отпишите, пожалуйста, баг на support@viva64.com, мы попробуем разобраться.
                      +1
                      Примеры ошибок в коде:

                      			if (totalReadsCount <= 0 || totalReadsCount <= 0 || noOptimizedSessions <= 0 || noOptimizedViews <= 0)
                      				return;
                      


                      			SessionId = SessionId;
                      


                      method always returns false
                      		private bool FixRelativeUrl(ref string url, Uri frameUrl)
                      		{
                      			if (Uri.TryCreate(url, UriKind.Relative, out frameUrl))
                      			{
                      				try
                      				{
                      					var frameUriBuilder = new UriBuilder("http", frameUrl.ToString());
                      					url = frameUriBuilder.Uri.AbsoluteUri;
                      				}
                      				catch
                      				{
                      					return false;
                      				}
                      			}
                      			return false;
                      		}
                      


                      V3010 The return value of function 'Remove' is required to be utilized. FeedProcessor.cs 429
                                  if (str[str.Length - 1] == ',')
                                      str.Remove(str.Length - 1);
                      


                      break is unconditional
                      		foreach (string author in authors)
                                      {
                                          ...
                                          break;
                                      }
                      


                      0
                      Давно не следил за вашими постами, потому есть вопрос:
                      Могу ли я бесплатно проверить коммерческий проект с открытым кодом? Если да, то как мне это сделать? А можете ли это сделать вы и написать статью? :)
                        +1
                        Скачайте триал и проверьте проект — это самый лучше способ познакомиться с анализатором. Сделать мы можем. Можем даже ошибки поправить. Но «за дорого» :-).
                          0
                          Кажется, вы меня неправильно поняли. Я имею к этому проекту отношение только как пользователь. Ну и я имел ввиду в рамках обычного цикла статей, таких, как эта.

                          В любом случае, спасибо за ответ. А вот вашим, хм, клиентом, я смогу стать когда вы научите PVS-Studio в Objective-C/Swift :)
                          +4
                          Тоже проверил свои почти 2 миллиона строк. Нашел около 70 High/Medium ошибок, из них:
                          1. ~20 ложных (в основном V3029, V3002, V3005, V3019)
                          2. ~40 стилистических/устоявшихся (легаси код, поведение для совместимости, старый стиль кода и др.)
                          3. 3 низкоприоритетных бага (V3025, V3001)
                          4. 1 среднеприоритетный баг (V3003 — «if (A) { } else if (A) { }»)
                          5. 2 высокоприоритетных (V3041 — целочисленное деление с результатом 0)

                          Удивительно, что последние два не нашел решарпер, хотя случаи очень простые. Спасибо.
                            0
                            Спасибо за подробную статистику! Однако не забывайте, что польза от статического анализа — в регулярном использовании.
                              +2
                              Забыл багрепорт/пожелание. В high-dpi (scaling = 175%) иконки слишком маленькие:
                            0
                            Небольшой вопрос возник:
                            Предупреждение PVS-Studio: V3004 The 'then' statement is equivalent to the 'else' statement. GetSemanticInfoTests.cs 2269
                            Смущает именование выделенных частей, почему не if statement?
                              +2
                              If <expression> then <action> else <action>

                              Т.е. ветки выполнения называются then и else, а if — это условие.
                                0
                                Ну да, логично же. А вот будь проблема в условии, вот тогда бы if statement был.
                              +3
                              В своем проекте триалом нашел 2 серьезные ошибки, остальное ложные срабатывания, но эти 2 ошибки стоят аплодисментов.
                                +1
                                Расскажите что за ошибки.
                                  +1
                                  Расскажите, пожалуйста, про ошибки. Статистика по диагностикам очень поможет нам при принятии решений по доработке существующих правил и разработке новых.

                                  Например, интересная статистика была в этом комментарии: habrahabr.ru/company/pvs-studio/blog/273811/#comment_8714091
                                    0
                                    Сразу скажу, я использовал пререлизную версию приложения.

                                    Первая ошибка связана с ошибкой в условии следующего вида.

                                    var i = 2;
                                    if(i < 0 && i > 2)…

                                    Вторая ошибка это присвоение значения поля/переменной самой себе.

                                    fieldName = fieldName;

                                    Т.к. места где ошибки были обнаружены отвечают за очень редкие кейсы, ошибки не были обнаружены ранее, но могли привести к неприятным проблемам у пользователей.

                                    Все остальное ложные срабатывания, так же было одно ошибочное о нем сразу сообщил разработчикам.

                                    Дмитрий, я уверен, что в низком количестве ошибок не малая заслуга вашего продукта :)

                                    –1
                                    Было бы круто, если бы ваш анализатор выкупил Microsoft за очень большие деньги.
                                      +1
                                      Как Скайп? Не, спасибо.
                                      Лучше бы просто содействовали в совершенствовании и интеграции, как в случае с Python-Tools
                                        0
                                        Как со Скайпом не будет. Включат в Visual Studio. А это совсем другая группа команд.
                                      +2
                                      После этой статьи решил-таки попробовать. И кое-что интересное нашёл :)
                                      source.OrderBy(String.Format("{0}.{1}", source.Name, MembersOf<TSource>.GetPath(keySelector), new ObjectParameter[] { }));
                                      


                                      И как должно быть:
                                      source.OrderBy(String.Format("{0}.{1}", source.Name, MembersOf<TSource>.GetPath(keySelector)), new ObjectParameter[] { });
                                      


                                      Т.е. у обеих функций одинаковая сигнатура: string format, params object [] и закрывающая скобка стояла не там. Сам даже если бы вчитывался, не заметил бы.
                                      V3025 Incorrect format. A different number of format items is expected while calling 'Format' function.

                                      Это очень здорово, спасибо команде :)
                                        0
                                        к счастью, в C# 6 можно теперь использовать $"{}"
                                          0
                                          Да, в новых проектах выручает сильно
                                        0
                                        Раз уж проверили Roslyn, то было бы также интересно узнать про ошибки в ANTLR C# Runtime.
                                          0
                                          Новая проверка: habr.com/ru/company/pvs-studio/blog/446592

                                          Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

                                          Самое читаемое