当前位置:坤哥网-kwan-sonar 扫出 c# 不合理的代码分析

sonar 扫出 c# 不合理的代码分析

2021/6/25 16:21:56 IT综合阅读(82) 评论(0)

  • sonar 扫出 c# 不合理的代码分析

    • 2.1 Optional parameters should not be used

    • 2.2 Remove this unused method parameter "XXOO"

    • 1.1 Change this condition so that it does not always evaluate to "true"

    • 1.2 Define the locale to be used in this string operation

    • 1.3 Remove this useless assignment to local variable "XXOO"

    • 一、bug类型

    • 二、坏味道类型

一、bug类型

1.1 Change this condition so that it does not always evaluate to "true"

规则描述:

Conditional statements using a condition which cannot be anything but false have the effect of making blocks of code non-functional. If the condition cannot evaluate to anything but true, the conditional statement is completely redundant, and makes the code less readable.It is quite likely that the code does not match the programmer's intent.Either the condition should be removed or it should be updated so that it does not always evaluate to true or false.Noncompliant Code Examplevar foo = true;// ...if (foo) // Noncompliant, always true{}

规则本身没问题,但造成了误报。

误报代码范例1:

private static DalUserFile _instance = null;public static DalUserFile Instance{
    get
    {
        if (_instance == null)
        {
            lock (_object)
            {
                if (_instance == null) _instance = new DalUserFile();
            }
        }
        return _instance;
    }}

这种实现方式叫双重检查锁,在处理多线程问题中经常使用。

1.2 Define the locale to be used in this string operation

规则描述:

string.ToLower(), ToUpper, IndexOf, LastIndexOf, and Compare are all culture-dependent, as are some (floating point number and DateTime-related) calls to ToString. Fortunately, all have variants which accept an argument specifying the culture or formatter to use. Leave that argument off and the call will use the system default culture, possibly creating problems with international characters.string.CompareTo() is also culture specific, but has no overload that takes a culture information, so instead it's better to use CompareOrdinal, or Compare with culture.Calls without a culture may work fine in the system's "home" environment, but break in ways that are extremely difficult to diagnose for customers who use different encodings. Such bugs can be nearly, if not completely, impossible to reproduce when it's time to fix them.Noncompliant Code Examplevar lowered = someString.ToLower(); //NoncompliantCompliant Solutionvar lowered = someString.ToLower(CultureInfo.InvariantCulture);orvar lowered = someString.ToLowerInvariant();

规则大致说的是担心出现国际字符问题。

代码范例:

if (ftpfilePath.IndexOf("/") > -1){
    ftpPath = ftpfilePath.Substring(0, ftpfilePath.LastIndexOf("/"));
    ftpName = ftpfilePath.Substring(ftpfilePath.LastIndexOf("/") + 1, ftpfilePath.Length - ftpfilePath.LastIndexOf("/") - 1);}

IndexOf、LastIndexOf、ToLower 等方法我们用的很多,系统已经用了这么久,没有因为这个问题发生错误,若要修改,涉及的代码比较多。

还有个不让【IndexOf 或 LastIndexOf】产生 bug 提示的一个方法是使用【char】入参,如【ftpfilePath.LastIndexOf('/')】,但这种情况只适于单个字符。

同样的提示还有【Culture should be specified for "string" operations】,代码范例:

var lowered = someString.ToLower(); //Noncompliant

应改成:

var lowered = someString.ToLower(CultureInfo.InvariantCulture);//或者var lowered = someString.ToLowerInvariant();

1.3 Remove this useless assignment to local variable "XXOO"

规则描述:

A dead store happens when a local variable is assigned a value that is not read by any subsequent instruction. Calculating or retrieving a value only to then overwrite it or throw it away, could indicate a serious error in the code. Even if it's not an error, it is at best a waste of resources. Therefore all calculated values should be used.Noncompliant Code Examplevoid CalculateRate(int a, int b){
  int i;

  i = a + b; // Noncompliant; calculation result not used before value is overwritten
  i = DoSomething();  // Noncompliant; retrieved value not used
  for (i = 0; i < 10; i++)
  {
    //  ...
  }
  // ...}Compliant Solutionvoid CalculateRate(int a, int b){
  int i;

  i = DoSomething();
  i += a + b;
  StoreI(i);

  for (i = 0; i < 10; i++)
  {
    //  ...
  }}

规则本身没问题,但造成了误报。

误报范例1(Remove this useless assignment to local variable "result"):

public string GetDB(string db){
    string result = string.Empty;
    using (var sqlConn = GetConnection(db))
    {
        result = sqlConn.Database;
    }
    return result;}

误报范例2(Remove this useless assignment to local variable "error"):

string error = string.Empty;CommonTool.UploadFileSftp(this.vfpsftp, ftpFilePath, pdfBase64, out error);

误报范例3(Remove this useless assignment to local variable "str3".)(Remove this useless assignment to local variable "str4"):

string str3 = string.Empty;    //从原num值中取出的值 string str4 = string.Empty;    //数字的字符串形式int temp;            //从原num值中取出的值 ...for (i = 0; i < j; i++){
    str3 = str4.Substring(i, 1);          //取出需转换的某一位的值 
    temp = Convert.ToInt32(str3);      //转换为数字
    ... }

误报范例4(Remove this useless assignment to local variable "ResponseByte"):

/// <summary>/// 提取网页Byte/// </summary>/// <returns></returns>private byte[] GetByte(){
    byte[] ResponseByte = null;
    using (MemoryStream _stream = new MemoryStream())
    {
        //GZIIP处理
        if (response.ContentEncoding != null && response.ContentEncoding.Equals("gzip", StringComparison.InvariantCultureIgnoreCase))
        {
            //开始读取流并设置编码方式
            new GZipStream(response.GetResponseStream(), CompressionMode.Decompress).CopyTo(_stream, 10240);
        }
        else
        {
            //开始读取流并设置编码方式
            response.GetResponseStream().CopyTo(_stream, 10240);
        }
        //获取Byte
        ResponseByte = _stream.ToArray();
    }
    return ResponseByte;}

其实 ResponseByte 在 using 中使用了的,按 sonar 规则认为应该定义在 using 代码块中就不会报提示,但是在 c# 中这样使用也很普遍,不管是否在using中。

但是即使将变量 ResponseByte 放到 using 中还是产生这个提示:

/// <summary>/// 提取网页Byte/// </summary>/// <returns></returns>private byte[] GetByte(){        
    using (MemoryStream _stream = new MemoryStream())
    {
        byte[] ResponseByte = null;
        //GZIIP处理
        if (response.ContentEncoding != null && response.ContentEncoding.Equals("gzip", StringComparison.InvariantCultureIgnoreCase))
        {
            //开始读取流并设置编码方式
            new GZipStream(response.GetResponseStream(), CompressionMode.Decompress).CopyTo(_stream, 10240);
        }
        else
        {
            //开始读取流并设置编码方式
            response.GetResponseStream().CopyTo(_stream, 10240);
        }
        //获取Byte
        ResponseByte = _stream.ToArray();
    }
    return new byte[0];}

不过可将代码改成如下就不会提示:

/// <summary>/// 提取网页Byte/// </summary>/// <returns></returns>private byte[] GetByte(){        
    using (MemoryStream _stream = new MemoryStream())
    {
        //GZIIP处理
        if (response.ContentEncoding != null && response.ContentEncoding.Equals("gzip", StringComparison.InvariantCultureIgnoreCase))
        {
            //开始读取流并设置编码方式
            new GZipStream(response.GetResponseStream(), CompressionMode.Decompress).CopyTo(_stream, 10240);
        }
        else
        {
            //开始读取流并设置编码方式
            response.GetResponseStream().CopyTo(_stream, 10240);
        }
        return  _stream.ToArray();
    }}

二、坏味道类型

2.1 Optional parameters should not be used

规则描述:

The overloading mechanism should be used in place of optional
parameters for several reasons:Optional parameter values are baked into the method call sitecode, thus, if a default value has been changed, all referencing
assemblies need
to be rebuilt, otherwise the original values will be used.The Common Language Specification (CLS) allows compilers to
ignore default parameter values, and thus require the caller to
explicitly specify
the values.The concept of optional argument exists only in VB.Net and C#.In all other languages like C++ or Java, the overloading
mechanism is the only
way to get the same behavior.Optional parameters prevent muddying the definition of the
function contract. Here is a simple example: if there are twooptional parameters,when one is defined, is the second one still optional ormandatory?Noncompliant Code Examplevoid Notify(string company, string office = "QJZ") // Noncompliant{}Compliant Solutionvoid Notify(string company){Notify(company, "QJZ");}void Notify(string company, string office = "QJZ"){}Exceptions
The rule ignores parameters with caller info attributes.

代码范例:

/// <summary>/// 获取配置的值/// </summary>/// <param name="key"></param>/// <param name="defaultVal"></param>/// <returns></returns>public string GetCfgVal(string key,string defaultVal=""){
    return config.GetProperty(key, defaultVal);}

按规则提示应改成:

/// <summary>/// 获取配置的值/// </summary>/// <param name="key"></param>/// <param name="defaultVal"></param>/// <returns></returns>public string GetCfgVal(string key){
    return config.GetProperty(key, "");}/// <summary>/// 获取配置的值/// </summary>/// <param name="key"></param>/// <param name="defaultVal"></param>/// <returns></returns>public string GetCfgVal(string key, string defaultVal){
    return config.GetProperty(key, defaultVal);}

使用默认参数【object a=...】这种模式在c#中很常见,我们系统中也大量这样使用,这是c#一种语法糖,如果要改成规则需要的模式,反而增加了代码量。

2.2 Remove this unused method parameter "XXOO"

规则描述:

Unused parameters are misleading. Whatever the value passed to such parameters is, the behavior will be the same.Noncompliant Code Examplevoid DoSomething(int a, int b) // "b" is unused{
  Compute(a);}void DoSomething2(int a) // value of "a" is unused{
  a = 10;
  Compute(a);}Compliant Solutionvoid DoSomething(int a){
  Compute(a);}void DoSomething2(){
  var a = 10;
  Compute(a);}Exceptionsvirtual, override methods and interface implementations are ignored.override void DoSomething(int a, int b) // no issue reported on b{
  Compute(a);}Furthermore, the this parameter of extension methods is also ignored.public static class Extensions{
  public static void MyHelper(this HtmlHelper helper) //no issue reported here
  {
    // no use of helper here
  }}Methods that have attributes defined on them are also ignored.public class MyDto{
  public string Name { get; set; }

  [OnDeserialized]
  private void OnDeserialized(StreamingContext context)
  {
    // ...
  }}

范例1(提示:Remove this unused method parameter "sender"):

private void OnChanged(object sender, ConfigChangeEventArgs changeEvent){...}private ApolloConfigUtility(string namespaceName){
    config = ConfigService.GetConfig(namespaceName);
    config.ConfigChanged += new ConfigChangeEvent(OnChanged);}

这是Apollo框架,其提供的委托 public delegate void ConfigChangeEvent(object sender, ConfigChangeEventArgs args) 需要传sender(事件源),我们要使用就必须定义一个包含该参数的方法private void OnChanged(object sender, ConfigChangeEventArgs changeEvent),即使这个参数sender不使用。


标签: 代码检查 sonar_
分类: 代码健康

有话要说? =>【不用注册,直接登录】,然后刷新本页面来发表您的观点(●'◡'●)