[Bug]: hangulIncludes 불변성(invariant)

[sa02045]

Bug description

hangulIncludes 함수는 올바르지 않은 불변성(invariant)을 가집니다.
이로인해 함수의도와 다른 결과값을 리턴합니다.

// case1: 한글이 아닌 문자열인 경우에도 true 반환

hangulIncludes('123', '12'); // true
hangulIncludes('abc', 'ab'); // true
hangulIncludes('你好', '你'); // true


// case2: 배열인 경우에도 true 반환

hangulIncludes(['12', '123', '123123'], ['12']); // true


// case3: 일부만 한글 문자열인 경우에도 true 반환

hangulIncludes('{"a": "가나다라"}', '가나다라'); // true

원인

함수 hangulIncludes는 "항상 x, y는 한글 문자열이다"라는 가정으로 작성되었습니다. 즉, 불변성(invariant)을 가집니다.

• 참고 What is an invariant?
  

function hangulIncludes(x: string, y: string){
   // x, y는 한글 문자열이라는 가정으로 작성
}

하지만 함수 hangulIncludes 내부에는 불변성을 검증하는 코드가 없습니다.

때문에 한글 문자열이 아닌 데이터를 전달받는 경우 의도하지않은 결과값을 리턴합니다.(런타임, 컴파일타임 모두)

함수 hangulIncludes는 유저 입력처럼 런타임에 데이터가 결정되는 경우에 자주 사용될 것으로 기대합니다.

정적인 데이터를 검증하는 케이스는 무의미하기 때문에 적을 것입니다.

다양한 케이스가 존재하는 동적인 런타임에서는 잘못된 불변속성으로 예상치 못한 버그를 발생시킬 수 있습니다.

hangulIncludes(dataFromServer, userInput) // ??

Expected behavior

No response

To Reproduce

https://codesandbox.io/p/sandbox/toss-es-hangule-xhcp7s?file=%2Fsrc%2FApp.tsx

Possible Solution

두가지 방법으로 접근할 수 있습니다.

1. 구현 단순성
2. 불변성 보장

구현 단순성

구현 단순성을 위해 https://github.com/toss/es-hangul/issues/32과 같은 접근방식이 있습니다.

hangulIncludes 함수는 한글 문자열을 다루는 것에만 집중하고 함수를 소비하는 곳에서 한글여부를 검증하는 방법입니다.

// x,y 가 동적인 데이터인 경우

if( isHangul(x) && isHangul(y) ) {
    hangulIncludes(x, y)
}

불변성 보장

불변성을 보장하기 위해 다음 방법이 있습니다.

1. assert를 활용한 에러 throw
2. boolean 리턴

assert

이미 존재하는 binaryAssembleHangulCharacters 함수와 마찬가지로 assert를 활용하여 한글문자열이 아닌 경우 조기에 에러를 throw 하는 방법입니다.

function binaryAssembleHangulCharacters(source: string, nextCharacter: string) {
  assert(
    isHangulCharacter(source) || isHangulAlphabet(source),
    `Invalid source character: ${source}. Source must be one character.`
  );
  assert(
    isHangulAlphabet(nextCharacter),
    `Invalid next character: ${nextCharacter}. Next character must be one of the chosung, jungsung, or jongsung.`
  );

// 생략...
// 생략...
}

hangulIncludes('123', '12'); // Error
hangulIncludes('abc', 'ab'); // Error
hangulIncludes('你好', '你'); // Error
hangulIncludes('{"a": 1}', 'a'); // Error
hangulIncludes(['12', '123', '123123'], ['12']); // Error

함수 소비자에게 에러처리와 같은 복잡한 케이스를 고려하게 만듭니다.

boolean return

에러를 던지지 않고 boolean값을 리턴합니다.

hangulIncludes('123', '12'); // false
hangulIncludes('abc', 'ab'); // false
hangulIncludes('你好', '你'); // false
hangulIncludes('{"a": 1}', 'a'); // false
hangulIncludes(['12', '123', '123123'], ['12']); // false

이것은 자바스크립트 내장 메서드인 String.includes() 동일한 인터페이스입니다. 단순성을 지니는 디자인입니다.

"string".includes([1,2,3]) // false
"string".includes({'a':'a'}) // false

etc.

[youngjae99]

저는 Parse, don't validate 글에서 말하는 원칙이 생각나는데요. 입력값을 미리 검증(validate)하기보다는 이를 파싱(parse)하여 유효한 형태로 변환하거나, 파싱 과정에서 오류가 발생하면 이를 처리하는 방식으로 접근하면 어떨까 싶습니다.

export function hangulIncludes(x: string, y: string) {
  // 한글 문자가 아닌 문자 제거
  const parseHangul = (str: string): string => str.replace(/[^가-힣]/g, '');
  
  // 한글 문자열로 파싱 후 disassemble
  const disassembledX = disassembleHangul(parseHangul(x));
  const disassembledY = disassembleHangul(parseHangul(y));

  return disassembledX.includes(disassembledY);
}

위와 같이 처리를 하게 되면

1. 별도의 유효성 검증 단계 없이 입력값을 파싱하여 단순하게 처리할 수 있고,
2. 함수가 한글 문자열만을 처리할 수 있도록 명확하게 파싱 과정을 통해 구현될 수 있다고 생각해요.

아래와 같은 여러 케이스의 입력값들을 유연하게 처리하면서, 함수의 목적에 맞게 동작할 것 같아요.

// 한글만 포함된 경우
hangulIncludes('안녕하세요', '하세요') // true
hangulIncludes('안녕하세요', '안녕') // true

// 한글과 다른 문자가 섞여있는 경우
hangulIncludes('안녕123하세요', '하세요') // false
hangulIncludes('hello안녕하세요', '안녕') // false
hangulIncludes('hello안녕하세요', 'hello') // false

// 한글이 아닌 경우
hangulIncludes('hello', '안녕') // false
hangulIncludes('123', '456') // false

[youngjae99]

idea) 적다보니 문자열에서 한글만 파싱(parseHangul)하는 함수가 라이브러리에서 제공되면 좋을 것 같네요. #108 에 관련 내용 올렸습니다.

[sa02045]

만약 파싱 과정에서 에러를 발생시키지 않고 싶다면, 아래처럼 zod의 safeParse와 같은 인터페이스를 가진 함수도 고려해볼만 한 것 같아요.

hangulIncludes 함수처럼 boolean을 return하는 조건확인함수의 경우 에러를 던지기보다 false를 return하는 것이 사용성이 편리하지 않을까 해서요

export function hangulIncludes(x: string, y: string) {
  const parsedHangulX = safeParseHangul(x)  
  const parsedHangulY = safeParseHangul(y)

  if( !(parsedHangulX.success && parsedHangulY.success) ){
     return false
  }
  
   // 한글 문자열로 파싱 후 disassemble
  const disassembledX = disassembleHangul(parsedHangulX.data);
  const disassembledY = disassembleHangul(parsedHangulY.data);
  
  // ...
}