소나큐브 코드스멜 테스트 결과 메시지 분석

 

 

 

SonarQube 에 GitLab&Local 연결해서 사용하기

 

위 포스트를 통해 소나큐브를 연결한 후 분석이 완료되었다면 아래와 같은 화면을 볼 수 있다.

 

 

이번 포스트에서는 발견된 결과들을 바탕으로 문제점들을 분석해볼 것이다.

 

 

 

 

 

---

 

 

 

 

 

 

버그

버그는 왠만하면 컴파일 과정 또는 작성 중 발견되기 때문에 개수가 많지는 않다.

 

 

1. Use try-with-resources or close this "Stream" in a "finally" clause.

 

Stream 자원을 사용하고 따로 close를 사용하여 닫아주지 않았기 때문에 발생한다.

try-with-resources 는 아래 포스트를 참고하면 된다.

JAVA의 try-with-resources

 

---

 

 

2. A "NullPointerException" could be thrown; "객체 변수명" is nullable here

 

변수에 repository 등을 통해 데이터를 초기화하여 null 값이 들어갈 위험이 있지만

별도의 예외 처리가 없이 변수의 데이터를 사용하려 할 경우 발생한다.

 

---

 

 

3. Change this condition so that it does not always evaluate to "false"

 

조건문이 언제나 false로 반환될 가능성이 있을 경우 발생한다.

 

조건문에서 체크되는 변수에 필수적으로 값이 삽입되게 설정되어있지 않은지 확인이 필요하다.

나의 경우 클래스명.clone() 메서드로 null일 경우 이미 오류가 발생해서 메서드가 종료될 상황이였기 때문에 발생했다.

 

---

 

 

4. "NullPointerException" will be thrown when invoking method "메서드명()".

 

메서드 호출 시 매개 변수에 null 값이 들어갈 가능성이 있는 경우 발생한다.

해당 매개 변수의 값이 null이 아닌지 확인하고 호출하는 로직이 필요하다.

 

---

 

 

5. A "List<String>" cannot contain a "int"

 

List<String> 에서 contains() 메서드를 통해 int 값을 찾으려할 때 발생한다.

int 형을 String.valueOf() 메서드를 통해 String 형으로 변경해주면 된다.

 

---

 

 

6. Call "Optional#isPresent()" before accessing the value.

 

객체가 null 일 경우를 고려하지 않고 값을 바로 사용하려고 하는 경우에 발생한다.

 

나의 경우 객체를 찾고 바로 get() 메서드를 통해 가져오려고 했기 때문에 발생했다.

NoSuchElementException이 발생할 수 있기 때문에 Optional을 사용해 null 값에 대비해야한다.

 

---

 

 

7. This class overrides "equals()" and should therefore also override "hashCode()".

 

재정의를 할 경우 equals()와 hashCode()를 둘 다 재정의 해야하지만 하나만 했을 경우 발생한다.

hashCode() 메서드를 재정의하지 않으면 해시 기반 자료구조(HashMap, HashSet 등)에서 문제가 발생할 수 있다.

 

---

 

 

8. Use another way to initialize this instance

 

익명 내부 클래스를 사용하여 HashMap을 초기화 할 경우 발생한다.

 

아래와 같이 익명으로 선언하여 바로 사용하는 것은 바람직 하지 않으므로,

Map<String, String> result = new HashMap<>() 와 같이 직접 변수를 선언한 후 데이터를 입력해주는 것이 바람직하다.

(직접 선언하는 것을 원하지 않을 경우 Java 9에 도입된 Map.of() 및 Map.ofEntries() 메서드를 사용하면 된다.)

// 변경 전
return new HashMap<String, String>(){{
    put(key, valueOperations.members(key));
}};

// 변경 후
Map<String, String> result = new HashMap<>(); 
result.put(key,"");
return result

 

 

 

 

 

---

 

 

 

 

 

 

취약점

버그까지의 단계는 아니고 개선이 필요한 부분을 알려주는 항목이다.

 

1. Make this "public static 변수명" field final

 

static으로 사용되는만큼 상수로 선언하는 것을 추천한다.

 

---

 

 

2. Make 변수명 a static final constant or non-public and provide accessors if needed.Use another way to initialize this instance

 

상수(static final)로 선언하거나 private으로 선언하고, 필요한 경우 접근자(get 등) 메소드 제공을 추천한다.

 

---

3. Use a logger to log this exception

 

Exception 부분에서 로그를 남기는 것을 추천한다.

 

---

4. Use Galois/Counter Mode (GCM/NoPadding) instead

 

기존에 AES/CBC/PKCS5Padding 암호화 방식을 사용했는데,

GCM 모드는 인증 암호화(Authenticated Encryption)를 제공하므로 데이터 무결성과 기밀성을 보장할 수 있다.

---

5. Make this IP "10.10.0.10" address configurable.

 

하드코딩을 하는 것이 아닌 환경 변수에 선언하는 것을 추천한다.

 

 

 

 

---

 

 

 

 

 

 

코드스멜

 

 

1. 2 duplicated blocks of code must be removed.

 

파일에 중복된 코드 블록이 하나 이상 있으면 발생한다.

 

---

2. Use isEmpty() to check whether the collection is empty or not.

 

나의 경우 조건문으로 List.size() == 0 을 입력해서 발생했다.

size를 가져와서 개수가 0인지 체크하는 것이 아닌 List.isEmpty() 메서드를 활용하는 것이 바람직하다.

 

---

3. Refactor this method to reduce its Cognitive Complexity from 32 to the 15 allowed.

 

인지 복잡성이 허용된 15를 초과하였을 경우 발생한다.

 

---

4. Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'.

 

메서드나 변수명이 정규표현식을 지키지 않고 선언되었을 경우 발생한다.

 

---

5. Declare "변수명" on a separate line.

 

변수가 같은 라인에서 초기화될 경우 발생한다.

각각 다른 라인에서 선언해주면 된다.

int total, expire = 0; // 변경 전

// 변경 후
int total = 0;
int expire = 0;

 

---

6. Define a constant instead of duplicating this literal "문자열" 5 times.

 

동일한 문자열이 여러 번 반복되어 사용될 경우 발생한다.

문자열을 상수로 선언해둔 후 사용해주면 된다.

 

---

7. Remove this "String" constructor 

Use a StringBuilder instead.

 

나의 경우 new String() 으로 변수를 초기화 해주어 발생했다.

 

문자열 생성 시 불필요한 메모리 할당이 발생할 수 있기 때문에 비효율적이라고 한다.

지속적으로 문자열이 추가될 경우에는 StringBuilder를 사용해주거나, "" 로 초기화 해주는 것을 추천한다.

---

8. Remove this unused "변수명" private field.

 

필드를 선언해놓고 사용하지 않을 때 발생한다.

이 메시지는 필드, 메서드 등 다양하게 나타난다.

---

9. Remove the "변수명" field and declare it as a local variable in the relevant methods.

 

변수를 전역 변수로 선언하는 것이 아닌, 필요한 메서드 내에 지역 변수로 선언할 것을 추천한다.

---

 

10. Extract this nested try block into a separate method.

 

중첩 try(try안에 try)를 사용할 경우 발생한다. (코드 가독성 저하, 유지보수 어려움, 성능 저하 이슈 발생 우려)

하나의 try에 여러 예외 처리를 설정해놓으면 된다.

---

 

11. Put single-quotes around '문자' to use the faster "indexOf(char)" method.

 

단일 문자를 찾지만, 문자열 메서드를 이용할 경우 발생한다.

 

lastIndexOf(char)는 lastIndexOf(String)보다 연산이 더 빠르다.

문자열 비교는 문자 비교보다 더 많은 연산이 필요하기 때문이다.

 

아래와 같이 싱글쿼트로 바꿔주면 된다.

 link.indexOf("/", subStart); // 변경 전
 link.indexOf('/', subStart); // 변경 후

---

 

12. Remove the declaration of thrown exception 'com.api.type.BizException' which is a runtime exception.

 

BizException은 RuntimeException을 상속받아 만든 자체 Exception 클래스이다.

 

RuntimeException의 하위 클래스이므로 메서드에서 명시적으로 선언할 필요가 없으며,

메서드 내에서 throw를 통해 Exception 처리를 하고있지만 throws와 중복 처리가 돼있을 경우에도 발생한다.

---

 

13. Make this final field static too.

 

final로 선언된 변수일 경우 static을 붙이는 것을 추천한다.

static을 추천하는 이유

메모리 효율성	static final 변수는 클래스 로딩 시 한 번만 메모리에 할당되고, 모든 인스턴스에서 공유된다. 이를 통해 메모리 사용을 최소화할 수 있다.
접근성 향상	static final 변수는 클래스 이름으로 직접 접근할 수 있어 코드 가독성과 유지보수성이 높아진다.
성능 향상	static final 변수는 컴파일 시 상수 폴딩(constant folding) 최적화 기법을 통해 더 빠른 실행 속도를 제공한다.

 

---

 

14. Replace this lambda with a method reference.

 

람다 표현식 내에 로직이 메서드 하나, 매개변수 하나만 활용하지만 메서드 참조형이 아닐 경우 발생한다.

 

이럴 경우 아래와 같이 메서드 참조형으로 변경해주면 된다.

cak.forEach((object) -> clientAgreementKeyRepository.delete(object)); // 변경 전
cak.forEach(clientAgreementKeyRepository::delete); // 변경 후

메서드 참조형의 장점

코드 가독성 향상	메서드 참조형은 람다 표현식보다 더 간결하고 이해하기 쉽다.
메모리 효율성	메서드 참조형은 람다 표현식보다 메모리 사용이 적다.
컴파일 최적화	메서드 참조형은 컴파일러가 더 잘 최적화할 수 있다.

 

---

 

15. Merge this if statement with the enclosing one.

 

중첩 if문이 존재 하는데 if 문 내부의 if문이 하나 밖에 없을 경우 발생한다.

이럴 경우 하나의 if문으로 합쳐주면 된다.

 

---

 

16. Replace the type specification in this constructor call with the diamond operator ("<>").

 

List 등 선언 시 타입 매개변수를 주었을 경우 발생한다.

타입 매개변수를 주었을 경우 의도치 않은 예외가 발생할 수 있으므로 생략해주는 것이 안전하다.

new ArrayList<Book>(4); // 변경 전
new ArrayList<>(4); // 변경 후

 

---

 

17. Replace the synchronized class "Hashtable" by an unsynchronized one such as "HashMap".

 

멀티스레드 환경이 아닌 경우 Hashtable 말고 HashMap을 사용하는 것을 추천한다.

HashMap은 동기화되지 않은 클래스로, 단일 스레드 환경에서 더 나은 성능을 보이며, 동기화로 인한 성능 저하를 방지할 수 있다.

 

 

---

 

18. Remove the parentheses around the "변수명" parameter

 

람다식에서 단일 매개 변수를 사용할 경우 괄호를 생략해야 한다.

(가독성 향상, 간결성 유지 목적, JAVA 8 이후 관용적으로 사용되는 표현 방식이다.)

괄호는 매개 변수가 2개 이상일 경우에만 사용한다.

books.forEach((val) -> ... ) // 변경 전
books.forEach(val -> ... ) // 변경 후

---

 

19. Remove useless curly braces around statement

 

람다식에서 단일 문장인데 중괄호({})를 사용할 경우 발생한다.
(가독성 향상, 간결성 향상 목적, JAVA 8 이후 관용적으로 사용되는 표현 방식이다.)

books.forEach(data -> {...}); // 변경 전
books.forEach(data -> ...); // 변경 후

// 여러 문장 - 중괄호 사용
books.forEach(data -> {
    문장1;
    문장2;
});

 

---

 

20. Immediately return this expression instead of assigning it to the temporary variable "변수명".

 

메서드에서 바로 리턴하지 않고 변수에 담은 직후 리턴할 경우 발생한다.

변수에 담은 직후 리턴하는 것이 좋지 않은 이유

불필요한 코드 생성	변수에 값을 할당한 후 바로 리턴하는 것은 불필요한 코드를 생성한다.
메모리 사용 증가	중간 변수를 생성하면 메모리 사용량이 증가한다.
성능 저하	불필요한 변수 생성과 할당 과정으로 인해 성능이 저하될 수 있다.
가독성 저하	중간 변수를 사용하면 코드가 복잡해져 가독성이 떨어질 수 있다.

 

---

 

21. Method has 9 parameters, which is greater than 7 authorized.

 

매개 변수가 7개를 초과할 경우 발생한다.

---

 

22. Complete cases by adding the missing enum constants or add a default case to this switch.

 

switch 문에서 열거형(enum) 타입을 이용하고 있지만, 모든 상수를 다루고 있지 않을 경우 발생한다.

(다루어지지 않은 상수가 나올 경우 오류가 발생할 수 있다.)

default를 사용하거나 모든 상수를 case로 적용시켜줘야 한다.

---

 

23. This block of commented-out lines of code should be removed.

 

주석 처리된 로직이 남아있을 경우 발생한다. (리소스 낭비)

---

 

24. Return an empty collection instead of null.

 

return null; 과 같은 코드가 있을 경우 발생한다. (NullException 발생 우려)

null을 return 하지말고 비어있는 객체를 return하는 것을 추천한다고 한다.

---

 

25. Change this instance-reference to a static reference.

 

객체의 static 필드에 인스턴스로 선언된 객체를 통해 접근하려고 할 때 발생한다.

 

static 멤버는 클래스에 속하는 것이지 인스턴스에 속하는 것이 아니기 때문이다.

따라서 static 멤버는 클래스 이름으로 직접 접근하는 것을 추천한다.

 

아래는 인스턴스로 선언해서 static 변수로 접근하는 잘못된 예이다.

A 클래스에 직접 접근하여 사용하는 것이 올바른 방법이다.

public class A {
	public static int counter = 0;
}

private A first = new A();
private A second = new A();

first.counter ++;
second.counter ++; // static 변수이므로 2가 되어있을 것이다.

A.counter ++; // 올바른 사용법

---

 

26. Replace this if-then-else statement by a single return statement.

 

조건문 내에 return 밖에 없고, return 값이 boolean일 경우 발생한다.

조건문 자체가 boolean 값을 뱉기 때문에 그 자체를 return으로 쓰는 것이 적합하다.

if (bookList.size() > 5) {
	return false; // 잘못된 사용법
} 

return bookList.size() > 5; // 올바른 사용법

---

 

27. Use "java.util.Random.nextInt()" instead.

 

Math.random() 을 이용해 랜덤 double 값을 생성하지만 실질적인 사용은 Int형을 원할 때 발생한다.

 

Math.random()은 0과 1 사이의 double 값을 생성하고 이를 n배하여 정수로 변환하는 과정이 필요하지만, Random.nextInt(n)은 직접 0과 n-1 사이의 정수를 생성한다.

 

또한, Math.random() * n은 편향된 결과를 생성할 수 있다.

예를 들어, Math.random() * 6은 0과 6 사이의 값을 생성하지만, 0에 가까운 값이 더 자주 생성될 수 있다.

(컴퓨터에서 부동 소수점 숫자는 유한한 정밀도를 가지므로, 0과 1 사이의 모든 실수를 균등하게 표현할 수 없다.)

반면 Random.nextInt(6)은 0부터 5까지의 값을 균등하게 생성합니다

---

 

28. This branch's code block is the same as the block for the branch on line 393.

 

동일한 코드를 여러번 사용했을 경우 발생한다.

393은 재사용된 코드라인을 뜻한다.

---

 

29. Iterate over the "entrySet" instead of the "keySet".

 

Map 에서 값을 뽑아와 사용할 때 entrySet이 아닌 keySet을 사용할 경우 발생한다.

map.entrySet()을 반복하는 것은 map.keySet()을 반복하는 것과 거의 동일한 비용이 든다.
그러나 map.entrySet()은 키와 값을 동시에 접근할 수 있어 코드가 더 간결해지고, 코드 변경 시 유지보수가 용이하다.

---

 

30. Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation.

 

메서드가 선언되었지만 안에 아무것도 구상되어있지 않을 경우 발생한다.

메서드가 비어 있는 이유를 설명하는 중첩 주석을 추가하거나, 메서드를 호출하면 UnsupportedOperationException을 던지도록 구현하면 된다.

/**
 * This method is currently empty.
 * 
 * /*
  * The reason this method is empty is because the functionality it was
  * originally intended to provide is no longer needed. However, the method
  * signature is being kept in case it needs to be implemented in the future.
  * If this method is called, it will throw an UnsupportedOperationException
  * to indicate that the operation is not currently supported.
  */
 public void someMethod() {
     throw new UnsupportedOperationException("This operation is not supported.");
 }

 

 

---

 

31. Remove this unnecessary cast to "클래스명".

 

불필요한 캐스팅이 있을 경우 발생한다.

 

아래 예시를 보면 clazz.getAnnotation(RequestMapping.class) 메서드는 이미 RequestMapping 타입의 객체를 반환하므로, 명시적인 캐스팅이 필요하지 않다.

// 변경 전
RequestMapping requestMapping = (RequestMapping) clazz.getAnnotation(RequestMapping.class);

// 변경 후
RequestMapping requestMapping = clazz.getAnnotation(RequestMapping.class);

---

 

32. Reorder the modifiers to comply with the Java Language Specification.

 

모디파이어 순서가 맞지 않을 때 발생한다.

Java 언어 명세(JLS)에 따라 모디파이어의 순서를 재정렬해주면 된다.

모디파이어 순서
1. 어노테이션
2. public, protected, private
3. abstract
4. static
5. final
6. transient
7. volatile
8. synchronized
9. native
10. strictfp

---

 

33. Define and throw a dedicated exception instead of using a generic one.

 

Error, RuntimeException, Throwable 같은 일반 예외 처리만 되어있을 경우 발생한다.

RuntimeException과 같은 일반적인 예외를 사용하는 것보다는 애플리케이션에 맞는 전용 예외를 정의하는 것이 좋다.

---

 

34. Add the "@Override" annotation above this method signature

 

부모 클래스의 메서드를 활용했지만 별도의 어노테이션을 표기하지 않았을 경우 발생한다.

@Override 어노테이션은 자식 클래스에서 부모 클래스의 메서드를 오버라이드했음을 명시적으로 선언하는 데 사용된다.
이를 통해 코드 가독성을 높이고 컴파일러가 오버라이딩 여부를 확인할 수 있다.

---

 

35. Add a private constructor to hide the implicit public one.

 

나의 경우 클래스명에 Utils가 붙었지만 public으로 선언되었을 때 발생했다.

 

클래스에 private 생성자를 추가하여 암시적인 public 생성자를 숨기는 방법이 있다.
이렇게 하면 클래스의 인스턴스화를 제한할 수 있으며, 클래스의 상태를 변경할 수 없도록 만들어 불변성을 보장할 수 있습니다.

// 변경 전
public class ActiveDirectoryUtils {}

// 변경 후
public class ActiveDirectoryUtils {
	private ActiveDirectoryUtils() {}
}

---

 

36. Remove this unnecessary null check; "instanceof" returns false for nulls.

 

나의 경우 한 변수를 null 이 아닌지 체크함과 동시에 특정 클래스가 맞는지 체크하는 로직이 있어서 발생했다.

 

instanceof 연산자는 null 값에 대해 자동으로 false를 반환하므로, 별도의 null 체크가 필요하지 않다.
따라서 null 체크를 제거하면 코드가 더 간결해지고 가독성이 향상된다.

response != null && response instanceof AccessAccept; // 변경 전
response instanceof AccessAccept; // 변경 후

---

 

37. Add the missing @deprecated Javadoc tag.

 

@Deprecated 어노테이션을 사용해 메서드가 사용되지 않음을 표시했지만 별도 주석처리가 없을 경우 발생한다.

 

@Deprecated 어노테이션을 사용할 때는 관련 Javadoc 태그인 @deprecated를 함께 사용하는 것이 좋다.

/**
* @deprecated 메서드 설명
* @since 지원 중단이 발생한 시기
* @forRemoval 향후 제거될지 여부
*/
@Deprecated
~

---

 

38. Extract this nested ternary operation into an independent statement.

 

중첩된 삼항연산자가 있을 경우 발생한다.

중첩된 삼항 연산자를 사용하면 코드가 복잡해지고 이해하기 어려워질 수 있다.
독립적인 if-else 문을 사용하면 각 조건을 명확하게 구분할 수 있고, 코드의 의도를 더 잘 파악할 수 있다.

---

 

39. Make "simpMessagingTemplate" transient or serializable.

 

클래스가 직렬화를 구현하지만 클래스에 의존성 추가된 클래스가 직렬화를 구현하지 않을 경우 발생한다.

 

나의 경우 simpMessagingTemplate 필드가 Serializable 인터페이스를 구현하는 클래스 내에 있기 때문에 발생했다.

첫 번째로, simpMessagingTemplate가 직렬화되지 않도록 transient 키워드를 사용하여 선언하는 방법이 있다.
이렇게 하면 객체가 직렬화될 때 simpMessagingTemplate 필드는 포함되지 않는다.
(역직렬화 시에는 기본값(null)이 할당된다.)

두 번째로, SimpMessagingTemplate 클래스를 Serializable 인터페이스를 구현하도록 변경하는 것이다.

 

직렬화의 개념은 아래 포스트에서 확인할 수 있다.

직렬화(Serialization) 란?

---

 

40. Remove the declaration of thrown exception 'Exception 경로', as it cannot be thrown from method's body.

 

메서드 내에 예외 처리가 되어있지만, 해당 예외가 발생할 가능성이 없을 경우에 표시된다.

해당 예외를 제거해주면 간단하게 해결된다.

---

 

41. Remove this "clone" implementation; use a copy constructor or copy factory instead.

 

나의 경우 @Override로 clone()을 구현했지만, CloneNotSupportedException에 대해선 적절한 처리가 없었기 때문에 발생했다.

 

CloneNotSupportedException은 클래스가 Cloneable 인터페이스를 구현하지 않았을 때 발생하므로, 이를 무시하면 예기치 않은 동작이 발생할 수 있다.

---

 

42. Remove this use of "기능명"; it is deprecated.

 

사용 중지될 기능이 있을 경우 발생한다.

---

 

43. Replace this "Map.get()" and condition with a call to "Map.computeIfAbsent()".

 

나의 경우 Map.get() 이후 조건문을 통해 메서드를 실행해서 발생했다.

 

Map.computeIfAbsent() 메서드를 사용하면 Map.get() 및 조건 검사를 단일 메서드 호출로 대체할 수 있다.
이를 통해 코드가 더 간결해지고 가독성이 향상된다.

// 변경 전
String value = map.get(key);
if (value == null) {
    value = computeValue(key);
    map.put(key, value);
}

// 변경 후
String value = map.computeIfAbsent(key, this::computeValue);

 

 

 

 

 

 

---

 

 

 

 

 

 

💡 TIP!

 

1. 소나큐브 사이트 한글 패치 하는 방법

 

아래 사이트를 클릭해보면 한글패치 JAR 파일을 다운로드 받을 수 있다.

소나큐브 버전 7까지만 지원하는 것 같다!

 

GitHub - SonarQubeCommunity/sonar-l10n-ko: SonarQube Korean Language Pack

아래에서 jar 파일 다운로드 받은 후 $SONAR_HOME/extensions/plugins/ 폴더에 넣어준다.

이 후 소나큐브를 재실행 시켜주면 한글 패치가 완료된다!

 

한글패치라해서 결과 메시지도 한글 패치가 되는 줄 알았지만 아래처럼 메뉴같은 항목만 한글 패치가 되고,

결과 메시지는 그대로 영어로 뜬다!