Sonarlint ile Refactoring

      S.A. Arkadaşlar,

      Bugünkü yazımız statik kod analizi yapan sonarqube alt yapısına bağlanan ve benzer kuralları işleten bir araç olan SonarLint'i inceleyeceğiz. SonarLint microsoft visual studio üzerine kurulan bir extention olup bunu rahatlıkla microsoft visual studio içinde kurabilir veya buradan da indirip kurabilirsiniz. Resmi sitesinde C# ile ilgili yaklaşık 390 kural görünmektedir, fakat biz sadece kendi karşılaştıklarımızı ele alacağız. Bunların bazılarını çözmek biraz daha kolay iken bazılarını çözmek ise biraz vakit alabiliyor. Tüm kurallar için ise buraya göz gezdirebilirsiniz. Karşılaştığım kuralları hangi kritere göre sıralamak konusunda çok düşündüm. En sonunda hata kodlarını küçükten büyüğe sıralamaya karar verdim. Bu yazıyı güncelle tutmaya özen gösterip karşılaştığımız hataları eklemeye devam edeceğiz.

Problem: S101 Rename class 'PDFConvertor' to match pascal case naming rules, consider using.

Çözüm: Pascal isimlendirmesine göre PdfConvertor yapmamız gerektiğini söylüyor.

Problem: S108 Either remove or fill this block of code.

try
{
   //code
}
catch { }

Çözüm: Try'in içini doldurmamız gerektiğini belirtmektedir. Sadece catch ile ilgili bir durum olmayıp diğer boş kod bloklarında yine aynı tavsiyeyi yapmaktadır.

Problem: S112 'System.Exception' should not be thrown by user code.

throw new Exception($"Cannot find template image content at: {path}");

Çözüm: Kendi exception yapımızı kullanmamız öneriliyor.

Problem: S125 Remove this commented out code.

Çözüm: Yorum satırlarını silmemiz tavsiye edilmektedir.

Problem: S927 Rename parameter 'xVariable' to 'variable' to match the interface declaration.

Çözüm: Sınıftaki metod içinde kullanılan parametre isimlendirmesi interface ile eşleşmiyor.

Problem: S1066 Merge this if statement with the enclosing one.

if(formatType == LabelType.PDF)
{
    if(size > 85)
    {
    }
}

Çözüm: Yukarıdaki 2 if ifadesini birleştirebileceğimiz tavsiye edilmektedir.

if(formatType == LabelType.PDF && size>85)
{
}

Problem: S1075 - Refactor your code not to use hardcoded absolute paths or URIs.

Çözüm: URL ifadesini hardcoded olarak kullanmamak gerektiğini belirtiyor. Bunu farklı şekillerde yapabiliriz. Bunlardan biri de config dosyasında tutmaktır. 

Problem: S1116 Remove this empty statement. 

Console.WriteLine("Hello, world!");;

Çözüm: Kod satırı sonunda 2 virgül konulmasına gerek olmadığını ikaz ediyor.

Problem: S1118 Add a 'protected' constructor or the 'static' keyword to the class declaration.

Çözüm: sadece static metodlar içeren bir sınıf içerisinde sınıfı static olarak işaretlemizi veya protected yapmamızı öneriyor. Bunun nedeni nesne oluşturulmasını engellemek. static metodlar bilindiği üzere nesne oluşturmaksızın sınıf ismi ile çağrılırlar.

Problem: S1125Remove the unnecessary Boolean literal(s).

Çözüm: if (variable == true) gibi ifade yerine if(variable) kullanımını öneriyor.

Problem: S1128 Remove this unnecessary 'using'.

Çözüm: Gereksiz olan using kullanımlarını silmemizi öneriyor.

Problem: S1135 - Complete the task associated with this 'TODO' comment.

Çözüm: Todo kalan işlemleri tamamlamızı istiyor.

Problem: S1155 Use '.Any()' to test whether this 'IEnumerable<T>' is empty or not.

Çözüm: Count kullanmak yerine Any kullanmak hem performans hem okunurluk açısından daha iyi olacaktır. 

 service.Count() == 0 -> service.Any()

Problem: S1168 Return an empty collection instead of null.

if (string.IsNullOrEmpty(templateData))
    return null;

Çözüm: return Array.Empty<byte[]>

 return Array.Empty<byte[]>

Problem: S1172 Remove this unused method parameter 'variable'. 

private string Method(Object variable)
{
}

Çözüm: Methoda geçilen parametre hiç kullanılmadığını belirtmektedir. 

Problem: S1450 Remove the field '_log' and declare it as a local variable in the relevant methods.

 private readonly ILog _log;
 public Constructor(ILog xLog)
 {
    _log = xLog;
 }  

Çözüm: sadece constructor içinde tanımlanıp herhangi bir metodda kullanılmadığı için bu uyarı vermektedir. Ya bunu diğer metodlar kullanmamız gerekir veya yerel bir değişken yapmamız daha olacağını tavsiye etmektedir.

Problem: S1481 Remove the unused local variable 'result'.

Çözüm: result değişkeni hiç kullanılmamıştır. Onu silebileceğimiz belirtiliyor.

Problem:S1643 Use a StringBuilder instead. result = result + " " + item;

Çözüm: string işlemlerini + operatörü ile yapmak yerine bunu string builder ile daha performanslı yapabileceğimizi önermektedir. 

var bld = new StringBuilder();
bld.Append(result);
bld.Append(" ");
bld.Append(item);
string str = bld.ToString();

Problem: S1656 Remove or correct this useless self-assignment.

input.Address = input.Address;

Çözüm: Kendine atama işleminin yapıldığını belirtmektedir. 

Problem: S1854 Remove this useless assignment to local variable.

Çözüm: Bir değişkene bir atama yapıldıktan sonra ardından bir atama daha yapılırsa gereksiz olan atamayı kaldırmamızı tavsiye ediyor.

i = a + b; //bunu silmenizi öneriyor, çünkü bu işlem gereksizdir.
i = compute();

Problem: S1871 Either merge this case with the identical one on line 241 or change one of the implementations.

switch (i)
{
   case 1:
     console.log();
   case 2:
     console.log();
}

Çözüm: case'lerden biri başka bir case ile aynı değeri içeriyor. O yüzden case ifadeleri birleştirilebilir denmektedir.

switch (i)
{
   case 1:
   case 2:
     console.log();
}

Problem: S1905 Remove this unnecessary cast to 'int'. (int)(item.W)

Çözüm: item.W int iken tekrardan cast işlemi yapmaya gerek yoktur. item.W demek yeterli olacaktır.

Problem: S2259 'obje' is null on at least one execution path 

Çözüm: obje null olabilir, bu yüzden obje.variable tarzı bir tanımlama hata fırlatabilir. O yüzden bunu obje?.variable ile null check yapabiliriz.

Problem: S2344 Rename this enumeration to remove the 'Enum' suffix.

Çözüm: UserTypeEnum -> UserType

Problem: S2486 Handle the exception or explain in a comment why it can be ignored.

catch (Exception exc) 
{
}

Çözüm: catch ifadesini boş bırakılmamasını gerektiğini belirtmektedir.

Problem: S2583 Change this condition so that it does not always evaluate to 'false'; some subsequent code is never executed.

bool a = false;
if (a)
{
   // never executed
}

Çözüm:  Bir işlem ile birlikte değişecek bir değişken kullanmalıyız.

Problem: S2696 Make the enclosing instance method 'static' or remove this set on the 'static' field. 

Çözüm: static bir değişkeni static olmayan bir metodda kullanmakta ortaya çıkar. Ya metodu static hale getirmeliyiz veya değişkeni static olmaktan çıkarmalıyız.

 

Problem: S2699 Add at least one assertion to this test case.

Çözüm: Test metodlarında en az bir Assert ifadesi kullanılması gerektiğini belirtmektedir.

[TestMethod]
public void MyMethod_WhenSomething_ExpectsSomething()
{
   //testing codes
   Assert.IsTrue(true);
}

Problem: S2737 Add logic to this catch clause or eliminate it and rethrow the exception automatically.

catch (Exception)
{
    throw;
}

Çözüm: Sadece sadece throw atmak ile olmaz, log tutma gibi bir iş yapmalıyız. 

catch (Exception)
{
   logger.LogError(e);
   throw;
}

Problem: S2933 Make '_variable' 'readonly'.

Çözüm: Değişkene sadece constructor içinde değer ataması yapıldıysa bunu readonly olarak işaretlememizi istiyor.

readonly object _obj;

Problem: S2971 Drop 'Where' and move the condition into the 'FirstOrDefault'.

   .Where(c => c.Id == 5).FirstOrDefault();

Çözüm: Where ifadesinden sonra FirstOrDefault() kullanımına gerek yoktur. Bunu şu şekilde kullanabiliriz. 

   .FirstOrDefault(c => c.Id == 5);

Problem: S2997 Remove the 'using' statement; it will cause automatic disposal of 'variable'.

Çözüm: Disposable arayüzünü uygulayan metodları using ile kullanarak onların yaşam sürelerini Dispose() ile sonlandırmak lazım, fakat bunun istinası IDisposable arayüzünü return eden bir nesnedir. Çünkü return ifadesi çağrıldığı yerde onu dispose edersek exception fırlatmasına sebebiyet verebiliriz.

Problem: S3445 Consider using 'throw;' to preserve the stack trace.

catch (Exception ex)
{
    throw ex;
}

Çözüm: 

   Console.WriteLine(ex);//or logging
   throw;

Problem: S3247 Replace this type-check-and-cast sequence with an 'as' and a null check.

if(ce is CarrierException)
{
     var carrierException = (CarrierException)ce;   
}

Çözüm:

var carrierException = ce as CarrierException;
if (carrierException  != null)
{
  // code
}

Problem: S3400 Remove this method and declare a constant for this value. 

public string GetFinalMileReference()
{
    return "TEST3435435435345";
}

Çözüm: Metodun sabit bir değişken döndürmesi yerine bunu const olan bir değişken ile tanımlayıp o şekilde kullanmayı tavsiye etmektedir. 

 const string variable ="TEST3435435435345";

Problem: S3458 Remove this empty 'case' clause.

case LabelTypeEnum.FINAL_MILE:
default:
        carrierCode = string.Empty;

Çözüm: default üzerindeki case  kullanımına gerek yoktur. Son ifade olarak zaten default seçeneğine gelecektir.

Problem: S3963 Initialize all 'static fields' inline and remove the 'static constructor'.

static Repository()
{
    _connectionString = ConfigurationManager.ConnectionStrings["db"].ConnectionString;
}

Çözüm:

private static readonly string _connectionString = ConfigurationManager.ConnectionStrings["db"].ConnectionString;

Problem: S4136 Method overloads should be grouped together

Çözüm: Metoduların dönüş tiplerine göre gruplama/sıralama yapılmasını önermektedir. 

Problem: S4586 Do not return null from this method, instead return 'Task.FromResult<T>(null)', 'Task.CompletedTask' or 'Task.Delay(0)'.

public Task<IActionResult> ChangePassword([FromBody] ChangePasswordModel model)
{
    return null;
}

Çözüm: 

    return Task.FromResult<IActionResult>(null);

    Daha düzgün temiz kodlar yazmak dileğiyle...