声纳“对局部变量的无用赋值”解决方法?

Yas*_*che 1 java null sonar-runner sonarqube

我正在努力改进代码,并且从Sonar遇到了这个问题:

Remove this useless assignment to local variable "uiRequest"
Run Code Online (Sandbox Code Playgroud)

事实是,它不是没有用的,因为我只是在代码中使用它:

        // I am supposed to remove this
        UiRequest uiRequest = null;

        if("Party".equals(vauban.getName()))   {
            uiRequest = contextBuilder.buildContext(vauban);
        } else {
            // Maybe I could work my way around here ?
            throw new NamingException(
                    String.format(
                            "Hey %s, change your name to %s, thanks",
                            vauban.getName(), "Vauban"));
        }

        // Set the generated Id in the result of context builder
        MyOwnService response = callService(uiRequest, vauban);

        return response;
Run Code Online (Sandbox Code Playgroud)

声纳仍然告诉我“ uiRequest”是没有用的,为什么?不是,因为我不希望它为null时到达代码。我尝试初始化它(uiRequest = new UiRequest()),但它一直告诉我它没有用。

任何人都知道声纳为何会如此/如何纠正此问题?

sli*_*lim 7

您的问题简化为:

Foo x = null;

if(a()) {
     x = b();
} else {
     throw new Exception();
}

c(x);
Run Code Online (Sandbox Code Playgroud)

这段代码有两条潜在的路径:

  1. a()返回truex被分配b()然后c(x)被调用。
  2. a()返回false。异常被抛出并且c(x)不被调用。

这些路径都没有c(x)使用 的初始分配调用null。因此,您最初分配的任何内容都是多余的。

请注意,如果初始分配不是空值,这也将是一个问题。除非赋值的右边有副作用,否则任何赋值都是浪费。(声纳分析副作用)

这对声纳来说是可疑的:

  • 也许程序员期望第一个赋值有效果——它没有,所以也许这是一个错误。
  • 它还与代码清晰度有关——未来的代码阅读者可能会浪费时间想知道初始值的用途。
  • 如果右侧有计算,但没有副作用,那就是浪费计算。

您可以通过两种方式解决此问题:

首先只是删除= null, 离开Foo x;- Java 足够聪明,可以意识到所有路由都c(x)涉及赋值,所以这仍然会编译。

更好的是,c(x)进入块:

if(a()) {
     Foo x = b();
     c(x);
} else {
     throw new Exception();
}
Run Code Online (Sandbox Code Playgroud)

这在逻辑上是等价的,更整洁,并缩小了x. 缩小范围是一件好事。当然,如果你需要 x在更广的范围内,你可以不这样做。

另一种变体,在逻辑上也是等效的:

if(! a()) {
   throw new Exception();
}

Foo x = b();
c(x);
Run Code Online (Sandbox Code Playgroud)

...它对“提取方法”和“内联”重构反应良好:

throwForInvalidA(...);
c(b());
Run Code Online (Sandbox Code Playgroud)

使用最能传达您意图的方式。


Kev*_*inO 5

您可以通过以下方式避免出现警告,并可能使代码更具可读性:

// I am supposed to remove this
//    UiRequest uiRequest = null;  <-- remove

// invert the test here
    if(! "Party".equals(vauban.getName()))   {
        // Maybe I could work my way around here ?
        throw new NamingException(
                String.format(
                        "Hey %s, change your name to %s, thanks",
                        vauban.getName(), "Vauban"));
    } 

    // you are only using the variable in the call service; make
    //   final since reference should not change after assignment
    final UiRequest uiRequest = contextBuilder.buildContext(vauban);

    // Set the generated Id in the result of context builder
    MyOwnService response = callService(uiRequest, vauban);

    return response;
Run Code Online (Sandbox Code Playgroud)