목록을 받아 처리하는 코드가 가끔 죽었는데 조회 자체는 정상으로 성공하고 있었다. 결과가 있으면 Collection 을 돌려주고 없으면 [] 를 돌려주는 형태의 코드였다.
Table of contents
Open Table of contents
원인 — 결과가 없을 때만 나는 오류
호출부는 ->pluck() 을 부르고 있어서 배열이 넘어오면 그 자리에서 Error 로 죽는다. 결과가 없을 때만 죽으므로 결과가 있는 대부분의 경우에는 안 드러났다.
빈 컬렉션도 그 자체로 컬렉션이므로 애초에 그 조건을 넣을 이유가 없었다. 조건을 지우고 항상 같은 것을 돌려주게 하니 호출부에서 따로 처리할 것이 없어졌다.
조치 — 같은 모양 찾기
같은 패턴을 grep -rn "return \[\];" 으로 저장소 전체에서 검색해 보니 열한 곳이 나왔다. 하나씩 확인하니 셋은 정말 배열을 돌려주는 것이 맞았고 나머지 여덟이 형태가 갈리고 있었다.
여덟을 고치면서 : Collection 반환 타입을 함께 명시했다. 타입을 적으면 다른 것을 돌려줄 때 그 자리에서 걸리는데 전에는 타입이 없어서 무엇을 돌려주는지 코드를 읽어야 알 수 있었다.
검증 — 한 번에 못 붙인 타입
반환 타입이 없는 메서드를 세어 보니 여든 개 남짓이었다. 두 가지 방법이 있었는데 한 번에 다 붙이는 것과 고칠 때마다 하나씩 붙이는 것이다. 뒤쪽을 골랐다. 이유는 그만한 수를 한 번에 붙이면 배포 하나에 회귀 위험이 몰려서다. 대신 반년 동안 타입이 있는 메서드와 없는 메서드가 섞여 있는 것을 감수했고 반년쯤 지나니 대부분 붙어 있었다.
값이 없을 때 null 을 돌려주는 것은 그 자체로 맞았지만 호출부가 확인을 안 하는 자리가 있었다. PHPStan 을 붙여 그런 자리를 찾았는데 공식 문서대로 --level 은 기본이 0 이고 올려 가며 채택할 수 있어서 그렇게 고쳤고 처음부터 max 로 하면 오류가 수천 개라 아무것도 못 본다.
주의 — 이름으로 가른 두 조회
값이 없을 때 널을 줄지 예외를 던질지도 메서드 이름으로 갈랐다. 없을 수 있는 조회는 널을 돌려주고 반드시 있어야 하는 조회는 예외를 던진다.
findOrFail() 은 주문 번호로 찾는데 없으면 잘못된 요청이므로 예외를 던지고 findBy() 는 조건에 맞는 것을 찾는데 없으면 정상이므로 ?Order 로 null 을 준다. 그러면 호출하는 쪽이 이름만 보고 어느 쪽인지 알 수 있게 된다.
배경 — 확인을 강제하는 결과 객체
성공과 실패를 배열에 담아 돌려주는 코드도 있었는데 호출부가 매번 ['success'] 키를 확인해야 했다. 그 키를 안 보면 실패한 결과를 성공으로 다루게 된다.
그래서 Result 객체로 바꾸고 value() 를 꺼내려면 isOk() 를 먼저 확인하게 만들었다. 실패한 결과에서 값을 꺼내려 하면 예외가 나므로 확인을 건너뛸 수 없는 구조가 됐다.
정리
- 성공 경로에서도 돌려주는 형태가 갈리면 받는 쪽이 깨진다
- 결과가 없을 때만 다른 형태면 있을 때는 안 드러난다
- 항상 같은 것을 돌려주고 빈 것도 같은 형태로 준다
- 반환 타입을 명시하면 다른 것을 돌려줄 때 걸린다
- 값이 없을 때 널을 줄지 예외를 던질지 이름으로 가른다
- 성공과 실패를 배열로 돌려주면 확인을 안 하게 된다
- 꺼내려면 확인하게 만드는 구조로 바꾼다
- 정적 분석은 레벨을 올려 가며 붙인다