1015_NET7_Errors_ru/image1.png


.NET 7 зарелизился. Это хороший повод покопаться в исходниках, чтобы поискать ошибки и странные места. За комментариями по находкам обратимся к самим разработчикам .NET — кому знать код, как не им? Погнали!


Я анализировал релизный код .NET 7. Взять его можно на GitHub: ссылка.


Перед релизом было 2 выпуска RC (release candidate), а поэтому основные баги должны быть устранены. Но тем даже интереснее, просочилось ли что-нибудь "в прод".


На каждый подозрительный фрагмент кода я открыл issue на GitHub. Это помогло понять, какой код ошибочен, какой избыточен, и какие правки вносили разработчики.


Issue 1


Проверка внимательности — что не так с кодом ниже?


internal sealed record IncrementalStubGenerationContext(
  StubEnvironment Environment,
  SignatureContext SignatureContext,
  ContainingSyntaxContext ContainingSyntaxContext,
  ContainingSyntax StubMethodSyntaxTemplate,
  MethodSignatureDiagnosticLocations DiagnosticLocation,
  ImmutableArray<AttributeSyntax> ForwardedAttributes,
  LibraryImportData LibraryImportData,
  MarshallingGeneratorFactoryKey<
    (TargetFramework, Version, LibraryImportGeneratorOptions)
  > GeneratorFactoryKey,
  ImmutableArray<Diagnostic> Diagnostics)
{
  public bool Equals(IncrementalStubGenerationContext? other)
  {
    return    other is not null
           && StubEnvironment.AreCompilationSettingsEqual(Environment, 
                                                          other.Environment)
           && SignatureContext.Equals(other.SignatureContext)
           && ContainingSyntaxContext.Equals(other.ContainingSyntaxContext)
           && StubMethodSyntaxTemplate.Equals(other.StubMethodSyntaxTemplate)
           && LibraryImportData.Equals(other.LibraryImportData)
           && DiagnosticLocation.Equals(DiagnosticLocation)
           && ForwardedAttributes.SequenceEqual(other.ForwardedAttributes, 
                (IEqualityComparer<AttributeSyntax>)
                  SyntaxEquivalentComparer.Instance)
          && GeneratorFactoryKey.Equals(other.GeneratorFactoryKey)
          && Diagnostics.SequenceEqual(other.Diagnostics);
    }

    public override int GetHashCode()
    {
      throw new UnreachableException();
    }
}

Ответ

Код проверяет эквивалентность двух объектов: this и other. Однако в одном из выражений допустили ошибку, сравнив свойство DiagnosticLocation с собой же.


Так неправильно:


DiagnosticLocation.Equals(DiagnosticLocation)

Так правильно:


DiagnosticLocation.Equals(other.DiagnosticLocation)

Эту проблему я нашёл в классе LibraryImportGenerator (ссылка на GitHub). Чуть позже нашёл ещё 2 записи с такими же ошибками, но уже в других классах:



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


1015_NET7_Errors_ru/image2.png


В .NET 8 рассмотренный код сильно переписан, а для .NET 7 фикс пока не применялся. Подробности можно почитать в issue на GitHub.


Issue 2


internal static void CheckNullable(JSMarshalerType underlyingSig)
{
    MarshalerType underlying = underlyingSig._signatureType.Type;
    if (underlying == MarshalerType.Boolean
        || underlying == MarshalerType.Byte
        || underlying == MarshalerType.Int16
        || underlying == MarshalerType.Int32
        || underlying == MarshalerType.BigInt64
        || underlying == MarshalerType.Int52
        || underlying == MarshalerType.IntPtr
        || underlying == MarshalerType.Double
        || underlying == MarshalerType.Single // <=
        || underlying == MarshalerType.Single // <=
        || underlying == MarshalerType.Char
        || underlying == MarshalerType.DateTime
        || underlying == MarshalerType.DateTimeOffset
        ) return;
    throw new ArgumentException("Bad nullable value type");
}

Location: JSMarshalerType.cs, 387 (link)


Здесь два раза проверили переменную underlying на равенство MarshalerType.Single. Иногда за подобными одинаковыми проверками скрываются ошибки: условно, должны были проверить переменные left и right, но два раза проверили left. Примеры подобных ошибок из Open Source проектов собраны здесь.


Открыл issue на GitHub: link. В рассматриваемом случае из .NET 7 повезло — проверка просто оказалась избыточной.


Issue 3


public static bool TryParse(string text, out MetricSpec spec)
{
  int slashIdx = text.IndexOf(MeterInstrumentSeparator);
  if (slashIdx == -1)
  {
    spec = new MetricSpec(text.Trim(), null);
    return true;
  }
  else
  {
    string meterName = text.Substring(0, slashIdx).Trim();
    string? instrumentName = text.Substring(slashIdx + 1).Trim();
    spec = new MetricSpec(meterName, instrumentName);
    return true;
  }
}

Location: MetricsEventSource.cs, 453 (link)


Метод TryParse всегда возвращает одно и то же значение – true. Это странно. Посмотрим, где он используется:


private void ParseSpecs(string? metricsSpecs)
{
  ....
  string[] specStrings = ....
  foreach (string specString in specStrings)
  {
    if (!MetricSpec.TryParse(specString, out MetricSpec spec))
    {
      Log.Message($"Failed to parse metric spec: {specString}");
    }
    else
    {
      Log.Message($"Parsed metric: {spec}");
      ....
    }
  }
}

Location: MetricsEventSource.cs, 375 (link)


Возвращаемое значение метода TryParse используется как условие оператора if. Если не удалось распарсить строку specString, исходное значение должно логироваться. Иначе логируется полученное представление — spec —, и над ним выполняются ещё какие-то операции.


Проблема в том, что TryParse всегда возвращает true. Значит, then-ветвь оператора if никогда не выполняется, то есть парсинг всегда считается успешным.


Issue на GitHub: link.


В результате исправления TryParse превратился в Parse, а в вызывающем методе пропал оператор if. В TryParse заодно поменяли Substring на AsSpan.


1015_NET7_Errors_ru/image3.png


Кстати, этот же фрагмент кода я отмечал в прошлый раз, когда копался в исходниках .NET 6. Тогда в методах логирования был пропущен символ интерполяции:


if (!MetricSpec.TryParse(specString, out MetricSpec spec))
{
  Log.Message("Failed to parse metric spec: {specString}");
}
else
{
  Log.Message("Parsed metric: {spec}");
  ....
}

Подробнее про эту и подобную ошибки можно почитать в статье про проверку .NET 6 (issue 14).


Issue 4


Раз уж мы затронули тему методов со странными возвращаемыми значениями, рассмотрим ещё один:


public virtual bool TryAdd(XmlDictionaryString value, out int key)
{
  ArgumentNullException.ThrowIfNull(value);

  IntArray? keys;

  if (_maps.TryGetValue(value.Dictionary, out keys))
  {
    key = (keys[value.Key] - 1);

    if (key != -1)
    {
      // If the key is already set, then something is wrong
      throw System.Runtime
                  .Serialization
                  .DiagnosticUtility
                  .ExceptionUtility
                  .ThrowHelperError(
      new InvalidOperationException(SR.XmlKeyAlreadyExists));
     }

     key = Add(value.Value);
     keys[value.Key] = (key + 1);
     return true;               // <=
  }

  key = Add(value.Value);
  keys = AddKeys(value.Dictionary, value.Key + 1);
  keys[value.Key] = (key + 1);
  return true;                  // <=
}

Location: XmlBinaryWriterSession.cs, 28 (link)


Метод или возвращает true, или выбрасывает исключение, но никогда не возвращает false. Это уже публичный API, так что и спроса больше.


Посмотрим описание на learn.microsoft.com:


1015_NET7_Errors_ru/image4.png


Нестыковочка. Я также открыл issue на GitHub (ссылка), но на момент написания статьи новостей не было.


Issue 5


public static Attribute? GetCustomAttribute(ParameterInfo element, 
                                            Type attributeType, 
                                            bool inherit)
{
  // ....
  Attribute[] attrib = GetCustomAttributes(element, attributeType, inherit);

  if (attrib == null || attrib.Length == 0)
    return null;

  if (attrib.Length == 0)
    return null;

  if (attrib.Length == 1)
    return attrib[0];

  throw new AmbiguousMatchException(SR.RFLCT_AmbigCust);
}

Location: Attribute.CoreCLR.cs, 617 (link)


В этом фрагменте кода 2 раза проверяют одно и то же выражение — attrib.Length == 0: сначала как правый операнд оператора '||', затем как условное выражение оператора if.


Иногда это признак ошибки — хотели проверить одно, а проверили другое. Здесь повезло: вторая проверка оказалась избыточной, и разработчики её убрали.


Issue на GitHub: link.


1015_NET7_Errors_ru/image5.png


Issue 6


protected virtual XmlSchema? GetSchema()
{
  if (GetType() == typeof(DataTable))
  {
    return null;
  }
  MemoryStream stream = new MemoryStream();

  XmlWriter writer = new XmlTextWriter(stream, null);
  if (writer != null)
  {
    (new XmlTreeGen(SchemaFormat.WebService)).Save(this, writer);
  }
  stream.Position = 0;
  return XmlSchema.Read(new XmlTextReader(stream), null);
}

Location: DataTable.cs, 6678 (link)


Создали экземпляр типа XmlTextWriter, ссылку на него записали в переменную writer и сразу на следующей строке проверяют её на неравенство null. Проверка всегда будет давать true, значит условие здесь лишнее.


Нестрашно, но проверку можно убрать. Так и сделали (issue на GitHub):


1015_NET7_Errors_ru/image6.png


Issue 7


Тоже избыточный код, но чуть менее очевидный.


public int ToFourDigitYear(int year, int twoDigitYearMax)
{
  if (year < 0)
  {
    throw new ArgumentOutOfRangeException(nameof(year), 
                                          SR.ArgumentOutOfRange_NeedPosNum);
  }

  if (year < 100)
  {
    int y = year % 100;
    return (twoDigitYearMax / 100 - (y > twoDigitYearMax % 100 ? 1 : 0)) 
             * 100 + y;
  }
  ....
}

Location: GregorianCalendarHelper.cs, 526 (link)


Давайте проследим за тем, как уточняется значение переменной year по мере исполнения кода.


ToFourDigitYear(int year, int twoDigitYearMax)

year – параметр метода типа int. Значит, его значение находится в диапазоне [int.MinValue; int.MaxValue].


При исполнении кода первым делом встречается оператор if, в котором выбрасывается исключение:


if (year < 0)
{
  throw ....;
}

Если исключение не было выброшено, значит значение year лежит в диапазоне [0; int.MaxValue].


Дальше ещё один оператор if:


if (year < 100)
{
  int y = year % 100;
  ....
}

Если исполнение кода зашло в then-ветвь оператора if, значит значение year находится в диапазоне [0; 99]. Это уже приводит к интересному результату операции взятия остатка от деления:


int y = year % 100;

Значение year всегда меньше 100 (от 0 до 99). Как следствие, результат операции year % 100 всегда будет равен левому операнду – year. Получается, что y и year всегда равны.


Итого, или код избыточный, или здесь какая-то ошибка. Открыл issue на GitHub: код поправили, убрав переменную y.


Issue 8


internal ConfigurationSection
FindImmediateParentSection(ConfigurationSection section)
{
  ....
  SectionRecord sectionRecord = ....
  if (sectionRecord.HasLocationInputs)
  {
    SectionInput input = sectionRecord.LastLocationInput;
    Debug.Assert(input.HasResult, "input.HasResult");
    result = (ConfigurationSection)input.Result;
  }
  else
  {
    if (sectionRecord.HasIndirectLocationInputs)
    {
      Debug.Assert(IsLocationConfig, 
                   "Indirect location inputs exist 
                    only in location config record");
      SectionInput input = sectionRecord.LastIndirectLocationInput;
      Debug.Assert(input != null);
      Debug.Assert(input.HasResult, "input.HasResult");
      result = (ConfigurationSection)input.Result;
    }
    ....
  ....
}

Location: MgmtConfigurationRecord.cs, 341 (link)


Здесь придётся немного повозиться. Для начала посмотрим на второй оператор if:


if (sectionRecord.HasIndirectLocationInputs)
{
  Debug.Assert(IsLocationConfig, 
               "Indirect location inputs exist 
                only in location config record");
  SectionInput input = sectionRecord.LastIndirectLocationInput;
  Debug.Assert(input != null);
  Debug.Assert(input.HasResult, "input.HasResult");
  result = (ConfigurationSection)input.Result;
}

В переменную input записывается значение свойства LastIndirectLocationInput. После этого input проверяется в двух ассертах: на null (input != null) и наличие результата (input.HasResult).


Посмотрим на тело свойства LastIndirectLocationInput, чтобы понять, какое значение может быть записано в переменную input:


internal SectionInput LastIndirectLocationInput
  =>   HasIndirectLocationInputs 
     ? IndirectLocationInputs[IndirectLocationInputs.Count - 1] 
     : null;

Смотрите, с одной стороны, свойство может вернуть значение null. С другой — видно, что если HasIndirectLocationInputstrue, то возвращается IndirectLocationInputs[IndirectLocationInputs.Count — 1], а не явное значение null.


Вопрос вот в чём — может ли значение из коллекции IndirectLocationInputs быть равно null? Возможно — из этого кода непонятно. Кстати, тут могли бы помочь nullable-аннотации, но они включены не во всех проектах .NET.


Возвращаемся в if:


if (sectionRecord.HasIndirectLocationInputs)
{
  Debug.Assert(IsLocationConfig, 
               "Indirect location inputs exist 
                only in location config record");
  SectionInput input = sectionRecord.LastIndirectLocationInput;
  Debug.Assert(input != null);
  Debug.Assert(input.HasResult, "input.HasResult");
  result = (ConfigurationSection)input.Result;
}

Условное выражение — sectionRecord.HasIndirectLocationInputs. Это то же самое свойство, которое проверяется в LastIndirectLocationInput. Значит, LastIndirectLocationInput точно не возвращает явный null. Однако какое значение будет получено из IndirectLocationInputs и записано в input — непонятно.


Разработчики сначала проверяют, что input != null, и лишь затем смотрят наличие результата — input.HasResult. Выглядит нормально.


Теперь вернёмся в первый оператор if:


if (sectionRecord.HasLocationInputs)
{
  SectionInput input = sectionRecord.LastLocationInput;
  Debug.Assert(input.HasResult, "input.HasResult");
  result = (ConfigurationSection)input.Result;
}

Посмотрим на свойство LastLocationInput:


internal SectionInput LastLocationInput 
  =>  HasLocationInputs 
    ? LocationInputs[LocationInputs.Count - 1] 
    : null;

Оно написано по тому же принципу, что и LastIndirectLocationInput. Как и в предыдущем случае, в зависимости от флага (HasLocationInputs) возвращается или null, или значение из коллекции LocationInputs.


Возвращаемся к оператору if. Его условное выражение — свойство HasLocationInputs, которое проверяется и внутри LastLocationInput. Если зашли в тело if, значит LastLocationInput не может вернуть явный null. Может ли значение из коллекции LocationInputs быть null? Вопрос открытый. Если может, то и в input тоже будет записан null.


Как и в случае с первым рассмотренным if, выполняется проверка input.HasResult, а вот проверка input != null на этот раз отсутствует.


Ещё раз. Первый разобранный фрагмент кода:


SectionInput input = sectionRecord.LastIndirectLocationInput;
Debug.Assert(input != null);
Debug.Assert(input.HasResult, "input.HasResult");
result = (ConfigurationSection)input.Result;

Второй:


SectionInput input = sectionRecord.LastLocationInput;
Debug.Assert(input.HasResult, "input.HasResult");
result = (ConfigurationSection)input.Result;

Выглядит так, будто пропустили выражение Debug.Assert(input != null).


Я открыл issue на GitHub, где описал это и другие подозрительные места, связанные с null-проверками (их мы рассмотрим ниже).


Рассмотренный код исправлять не стали, оставив as is:


1015_NET7_Errors_ru/image7.png


Issues c проверкой на null


В коде я встретил несколько мест, в которых ссылка сначала разыменовывается, а затем проверяется на null. Чтобы не плодить задачи, собрал их все в общем issue на GitHub.


Давайте разберём эти места.


Issue 9


private static RuntimeBinderException BadOperatorTypesError(Expr pOperand1, 
                                                            Expr pOperand2)
{
  // ....
  string strOp = pOperand1.ErrorString;

  Debug.Assert(pOperand1 != null);
  Debug.Assert(pOperand1.Type != null);

  if (pOperand2 != null)
  {
    Debug.Assert(pOperand2.Type != null);
    return ErrorHandling.Error(ErrorCode.ERR_BadBinaryOps,
                               strOp, 
                               pOperand1.Type, 
                               pOperand2.Type);
  }

  return ErrorHandling.Error(ErrorCode.ERR_BadUnaryOp, strOp, pOperand1.Type);
}

Location: ExpressionBinder.cs, 798 (link)


Сначала pOperand1 разыменовывается (pOperand1.ErrorString), а уже на следующей строке проверяется на null в Debug.Assert. Если pOperand1 будет null, вместо срабатывания ассерта будет выброшено исключение типа NullReferenceException.


Код поправили, вынеся проверку pOperand1 до использования.


Было:


string strOp = pOperand1.ErrorString;

Debug.Assert(pOperand1 != null);
Debug.Assert(pOperand1.Type != null);

Стало:


Debug.Assert(pOperand1 != null);
Debug.Assert(pOperand1.Type != null);

string strOp = pOperand1.ErrorString;

Issue 10


public void Execute()
{
  var count = _callbacks.Count;
  if (count == 0)
  {
    return;
  }

  List<Exception>? exceptions = null;

  if (_callbacks != null)
  {
    for (int i = 0; i < count; i++)
    {
      var callback = _callbacks[i];
      Execute(callback, ref exceptions);
    }
  }

  if (exceptions != null)
  {
    throw new AggregateException(exceptions);
  }
}

Location: PipeCompletionCallbacks.cs, 20 (link)


Переменную _callbacks сначала используют, а затем проверяют её значение на неравенство null:


public void Execute()
{
  var count = _callbacks.Count;
  ....
  if (_callbacks != null)
  ....
}

На момент написания статьи код исправили, убрав проверку _callbacks на null.


Кстати, _callbacks – это readonly поле, которое инициализируется в конструкторе:


internal sealed class PipeCompletionCallbacks
{
  private readonly List<PipeCompletionCallback> _callbacks;
  private readonly Exception? _exception;
  public PipeCompletionCallbacks(List<PipeCompletionCallback> callbacks, 
                                 ExceptionDispatchInfo? edi)
  {
    _callbacks = callbacks;
    _exception = edi?.SourceException;
  }
  ....
}

В треде с исправлениями обсуждали, стоит ли добавить в конструктор Debug.Assert с проверкой _callbacks на null. В итоге решили не делать.


1015_NET7_Errors_ru/image8.png


Issue 11


private void ValidateAttributes(XmlElement elementNode)
{
  ....
  XmlSchemaAttribute schemaAttribute 
    = (_defaultAttributes[i] as XmlSchemaAttribute)!;
  attrQName = schemaAttribute.QualifiedName;
  Debug.Assert(schemaAttribute != null);
  ....
}

Location: DocumentSchemaValidator.cs, 421 (link)


Противоречивый код:


  1. Результат работы оператора as записывается в schemaAttribute. Если _defaultAttributes[i]null, или приведение выполнить не удалось, результатом будет null.
  2. null-forgiving оператор ('!') намекает, что результат приведения не может быть null. Как следствие, schemaAttribute не должен быть равен null.
  3. На следующей строке schemaAttribute разыменовывается. Ещё строкой ниже проверяется, что она не равна null.

Внимание, вопрос. Может ли schemaAttribute иметь значение null? Из этого кода, конечно, не очень понятно.


Исправили код так:


....
XmlSchemaAttribute schemaAttribute 
  = (XmlSchemaAttribute)_defaultAttributes[i]!;
attrQName = schemaAttribute.QualifiedName;
....

В обсуждении правок предлагали не удалять вызов Debug.Assert, а перенести его строкой выше. Код выглядел бы так:


....
XmlSchemaAttribute schemaAttribute = (XmlSchemaAttribute)_defaultAttributes[i]!;
Debug.Assert(schemaAttribute != null);
attrQName = schemaAttribute.QualifiedName;
....

В итоге Assert решили не возвращать:


1015_NET7_Errors_ru/image9.png


Issue 12


Взглянем на конструктор типа XmlConfigurationElementTextContent:


public XmlConfigurationElementTextContent(string textContent, 
                                          int? linePosition, 
                                          int? lineNumber)
{ .... }

Location: XmlConfigurationElementTextContent.cs, 10 (link)


Теперь посмотрим, где он используется:


public static IDictionary<string, string?> Read(....)
{
  ....
  case XmlNodeType.EndElement:
    ....
    var lineInfo = reader as IXmlLineInfo;
    var lineNumber = lineInfo?.LineNumber;
    var linePosition = lineInfo?.LinePosition;
    parent.TextContent = new XmlConfigurationElementTextContent(string.Empty, 
                                                                lineNumber,
                                                                linePosition);
    ....
    break;
  ....
  case XmlNodeType.Text:
    ....
    var lineInfo = reader as IXmlLineInfo;
    var lineNumber = lineInfo?.LineNumber;
    var linePosition = lineInfo?.LinePosition;

    XmlConfigurationElement parent = currentPath.Peek();

    parent.TextContent = new XmlConfigurationElementTextContent(reader.Value,
                                                                lineNumber, 
                                                                linePosition);
    ....
    break;
  ....
}

Locations:


  • XmlStreamConfigurationProvider.cs, 133 (link)
  • XmlStreamConfigurationProvider.cs, 148 (link)

Заметили подвох в коде?


Обратите внимание на порядок аргументов и параметров:


  • аргументы: ..., lineNumber, linePosition;
  • параметры: ..., linePosition, lineNumber.

Я открыл issue на GitHub (link), код поправили: аргументы поменяли местами и добавили тест.


Issue 13


Ещё один подозрительный случай:


public virtual bool Nested
{
  get {....}
  set 
  {
    ....
    ForeignKeyConstraint? constraint 
      = ChildTable.Constraints
                  .FindForeignKeyConstraint(ChildKey.ColumnsReference, 
                                            ParentKey.ColumnsReference); 
    ....
  }
}

Location: DataRelation.cs, 486 (link)


Посмотрим на метод FindForeignKeyConstraint:


internal ForeignKeyConstraint? 
FindForeignKeyConstraint(DataColumn[] parentColumns, 
                         DataColumn[] childColumns)
{ .... }

Location: ConstraintCollection.cs, 548 (link)


Похоже, порядок аргументов опять неверный:


  • параметры: parent..., child...
  • аргументы: ChildKey..., ParentKey...

Есть ещё один вызов этого метода: там к порядку аргументов нет вопросов.


ForeignKeyConstraint? foreignKey
  = relation.ChildTable
            .Constraints
            .FindForeignKeyConstraint(relation.ParentColumnsReference,
                                      relation.ChildColumnsReference);

Открыл issue на GitHub: link. Увы, на момент написания статьи я не получил комментариев по этому коду.


Issue 14


Это не все места, где порядок аргументов перепутан — нашёл ещё одно:


void RecurseChildren(....)
{
  ....
  string? value 
    =  processValue != null
      ? processValue(new ConfigurationDebugViewContext(
                           child.Key, 
                           child.Path, 
                           valueAndProvider.Value, 
                           valueAndProvider.Provider))
      : valueAndProvider.Value;

  ....
}

Location: ConfigurationRootExtensions.cs, 50 (link)


Посмотрим на конструктор ConfigurationDebugViewContext:


public ConfigurationDebugViewContext(
  string path, 
  string key, 
  string? value, 
  IConfigurationProvider configurationProvider) 
{ .... }

Location: ConfigurationDebugViewContext.cs, 11 (link)


Последовательность:


  • параметры: path, key, ...
  • аргументы: child.Key, child.Path, ...

Открыл issue на GitHub: link. Со слов разработчиков, несмотря на ошибку, этот кейс не вызывал проблем.


1015_NET7_Errors_ru/image10.png


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


Заключение


Код .NET 7 высокого качества. Я думаю, этому способствует в том числе налаженный процесс разработки: даты релизов известны, выпуски RC тоже проводятся не зря.


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


Все фрагменты кода, описанные в статье, я нашёл анализатором PVS-Studio. Да, теперь им можно проверять проекты на .NET 7.


Если захотите проверить свой проект (личный или коммерческий), анализатор можете загрузить с этой страницы. Там же есть ссылка на документацию: в ней мы описали, как ввести лицензию и запустить анализ. Если возникнут проблемы или вопросы — обязательно пишите нам, поможем.

Комментарии (11)


  1. Zara6502
    14.12.2022 11:24
    -2

    free версия анализатора не планируется?


    1. foto_shooter Автор
      14.12.2022 11:39
      +3

      Кушать хочется. :)

      Если хочется попробовать — проще взять триал.

      Если речь именно о регулярном бесплатном использовании — в этой статье разобрано несколько вариантов (для студентов, некоммерческих проектов и т. п.).


      1. Zara6502
        14.12.2022 11:45
        +2

        невозможно скушать, то что не продано.

        спасибо за статью, я думаю меня устроит вариант с ремарками в коде, тем более это всего 1 файл.


        1. foto_shooter Автор
          14.12.2022 11:52
          +3

          Всегда пожалуйста. Здорово, что удалось найти подходящий вариант использования. :)


  1. nronnie
    14.12.2022 15:49

    Было бы интересно прочитатать если бы вы опубликовали сравнение вашего анализатора, SonarCube, и Microsoft.CodeAnalysis.NetAnalyzers.


    1. foto_shooter Автор
      14.12.2022 17:21
      +2

      Вопрос сравнения хороший, спасибо.

      На самом деле коллеги уже проходили это на примере C++ анализатора. Ответ — в заметке “Почему мы не пишем о сравнении PVS-Studio с другими статическими анализаторами кода”.

      Однако мне захотелось переосмыслить тему сравнения.
      Комментарий получается большим, пришлось даже открыть Word, чтобы всё сформулировать. :)

      Чуть позже напишу его в этой же ветке.


    1. foto_shooter Автор
      14.12.2022 21:36
      +1

      Тема сравнения инструментов на самом деле сложная. :(

      Анализаторы — инструменты комплексные. То есть это не только про “выдать предупреждение”, но и про удобство интеграции, производительность, разные доп. возможности (типа лёгкого старта) и т. п.

      Можно, конечно, всё свести к одним только возможностям анализа: чтобы предупреждения выдавал и не фолсил. Однако я думаю, это будет не совсем корректно.

      Если всё-таки браться сравнивать инструменты исключительно по тем предупреждениям, которые они выдают — это тоже та ещё работёнка. Просто сравнить количество предупреждений — не вариант. Нужно тщательно анализировать вывод: смотреть true positives, false positives, а по-хорошему — и false negatives (хотя бы отталкиваясь от true positives других инструментов).

      Окей, допустим, решили посмотреть true positives. Тут тоже начинаются интересности: одну и ту же ошибку разные инструменты скорее будут всего подсвечивать разными сообщениями, а вероятно — ещё и выдавать их на разные строки. Этой темы я касался в докладе "Как устроено тестирование средства статического тестирования", когда рассказывал про тестирование на бенчмарках.

      Допустим, сделали сравнение, потратили кучу сил и времени.

      Побеждает PVS-Studio. Какая будет реакция? Скорее всего что-то вроде: “Ага, ну конечно. Статью же люди из PVS-Studio писали — какие неожиданные результаты…”.

      Побеждает не PVS-Studio. Тут у меня, как заинтересованного лица, появляется сильное желание улучшить анализатор, чтобы он показал себя лучше. Почти 8 лет жизни в продукт вложено — шутка ли? :)

      Получается, что я пришёл примерно к тому же, что описано в первом пункте статьи “Почему мы не пишем о сравнении PVS-Studio с другими статическими анализаторами кода”. Это хорошо — кажется, она не потеряла своей актуальности. :)

      Как же быть?

      Если рассматривать вопрос с прагматической точки зрения, я бы переформулировал его так: "Какой из анализаторов принесёт больше пользы на нашем проекте: A, B или C?"

      Как это понять? Попробовать. :)

      P.S. Если решите попробовать PVS-Studio, рекомендую полистать заметку: "PVS-Studio: 2 фишки для быстрого старта". Она небольшая, но рассказывает о двух важных механизмах, которые сделают знакомство приятнее.

      P.P.S. И ещё стоит помнить о философии статического анализа.


  1. odisseylm
    14.12.2022 18:14
    +1

    Скорее всего многие из ошибок были подсвечены "warning"-ами самой средой разработки (или ее плагином). Но как говорится: "Я работаю как все - на отъебись". К сожалению, с таким отношением к коду/работе частенько сталкиваюсь :-(


    1. foto_shooter Автор
      14.12.2022 21:12
      +1

      Скорее всего многие из ошибок были подсвечены "warning"-ами самой средой разработки (или ее плагином).

      Когда я разбирал логи анализатора, то работал в Visual Studio. Насколько я помню, в текстовом редакторе она ни одного из выписанных мест не подсветила.

      Что из этого детектит Rider, а что нет — сказать не могу. В любом случае, факт в том, что всё описанное ушло в релиз. :)

      К сожалению, с таким отношением к коду/работе частенько сталкиваюсь :-(

      Интересно, если поделитесь мыслями на этот счёт: что с этим делать?


      1. odisseylm
        16.12.2022 05:11

        >> Когда я разбирал логи анализатора, то работал в Visual Studio ... она ни одного из выписанных мест не подсветила.

        Я джавист и работаю в Idea, и она достаточно хороша (а можно еще и добавить/включить дополнительные плагины), но люди кладут прибор на ее warnings (компилится, да еще и работает)

        >> Интересно, если поделитесь мыслями на этот счёт: что с этим делать?

        Не знаю, к сожалению :-( Если вы тимлид и имеете власть, то можете попробовать включить TreatWarningsAsErrors у всех (как написал @nronnie), ну а также выставить жесткие правила на build-server (TeamCity/Jenkins/или что там у MS). А в целом это вопрос из разряда "Как заставить людей не мусорить/воровать и быть хорошими? :-) "


    1. nronnie
      14.12.2022 21:41
      +5

      "На от...бись" лечится флагом

      <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
      

      в файле Directory.Build.props всего солюшена :)