Skip to content

Conversation

takejohn
Copy link
Member

@takejohn takejohn commented Sep 12, 2025

What

識別子の処理に関するリファクタリングになります。

  • ソースファイルにsrc/utils/character.tsを追加しました。Unicodeなど文字に関するユーティリティ関数群です。test/characters.tsでテストを行います。
  • src/parser/streams/char-stream.tsCharStreamにおいて、サロゲートペアを1文字として扱うようにし、.charがUnicode文字を返すようにしました。.pos()は従来通りUTF-16コード単位ベースの値を返します。
  • test/keywords.tsにあった予約語のテストに、識別子に使用できる文字のテストを加え、新しいテストファイルをtest/identifiers.tsとしました。
  • test/parser.ts内のテスト'Scanner' > 'invalid token'のコードを$から~に変更しました。

Why

Additional info (optional)

@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.84979% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/parser/scanner.ts 88.57% 8 Missing ⚠️
src/parser/streams/char-stream.ts 91.11% 4 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Files with missing lines Coverage Δ
src/parser/plugins/validate-keyword.ts 100.00% <100.00%> (+11.38%) ⬆️
src/utils/characters.ts 100.00% <100.00%> (ø)
src/parser/streams/char-stream.ts 92.64% <91.11%> (+4.15%) ⬆️
src/parser/scanner.ts 94.99% <88.57%> (+6.22%) ⬆️

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@syuilo
Copy link
Collaborator

syuilo commented Sep 12, 2025

識別子の最初を除いた部分では、以下の文字も使用できます。
以下のUnicodeカテゴリに含まれる全ての文字
Non-spacing mark (Mn)
Combining spacing mark (Mc)
Decimal number (Nd)
Connector punctuation (Pc)
ゼロ幅非接合子 (ZWNJ)
ゼロ幅接合子 (ZWJ)

実際にJSONのキーとして使われることってまずない気がするから省いちゃって良さそうな気はしました

@takejohn
Copy link
Member Author

識別子の最初を除いた部分では、以下の文字も使用できます。
以下のUnicodeカテゴリに含まれる全ての文字
Non-spacing mark (Mn)
Combining spacing mark (Mc)
Decimal number (Nd)
Connector punctuation (Pc)
ゼロ幅非接合子 (ZWNJ)
ゼロ幅接合子 (ZWJ)

実際にJSONのキーとして使われることってまずない気がするから省いちゃって良さそうな気はしました

その中では、Decimal number (Nd)に関しては[0-9]を含むので残しておくか、[0-9]に限定したほうがいいかもしれないですね。
他は確かに使用頻度が少なさそうです。

識別子としてどのような文字列が書けたほうがいいんですかね?
[A-Z]はUppercase letter (Lu)、[a-z]はLowercase letter (Ll)に含まれます。
日本語など、ラテン文字以外で変数名を付けたい場合、Other Letter (Lo)が必要になると思います。

だから

  • Uppercase letter (Lu)
  • Lowercase letter (Ll)
  • Other letter (Lo)
  • $
  • _
  • Decimal number (Nd)

を残して他を省くといいですかね。

@syuilo
Copy link
Collaborator

syuilo commented Sep 12, 2025

AiScript的には識別子にはアルファベットと数字と一部の記号しか使えなくて良さそうと思ってます(=日本語などは使えない)

@takejohn
Copy link
Member Author

あれ?じゃあもうこのPRいらなかったですか?

@syuilo
Copy link
Collaborator

syuilo commented Sep 12, 2025

あー
ただ今後ずっとその仕様かどうかは分からないから文字種とは関係ない部分の変更は入れると良いのかも?

ソースファイルにsrc/utils/character.tsを追加しました。Unicodeなど文字に関するユーティリティ関数群です。test/characters.tsでテストを行います。

src/parser/streams/char-stream.tsのCharStreamにおいて、サロゲートペアを1文字として扱うようにし、.charがUnicode文字を返すようにしました。.pos()は従来通りUTF-16コード単位ベースの値を返します。

とか

@syuilo
Copy link
Collaborator

syuilo commented Sep 12, 2025

AiScript的には識別子にはアルファベットと数字と一部の記号しか使えなくて良さそうと思ってます(=日本語などは使えない)

ご意見募集

@syuilo
Copy link
Collaborator

syuilo commented Sep 12, 2025

まあ議論の余地がありそうだから実際に使用可能な文字種を増やす部分の変更は一旦削除かコメントアウトしてもらって、それ以外の変更は一足先に入れるで良いかもしれないわね

@marihachi
Copy link
Contributor

marihachi commented Sep 13, 2025

普段プログラムを書いてる人が対象となっているならば不要、そうでなければ使えても良いんじゃないでしょうか

@takejohn takejohn changed the title enhance: JSON5の識別子を使えるように refactor: 識別子の処理に関するリファクタリング Sep 13, 2025
@takejohn
Copy link
Member Author

まあ議論の余地がありそうだから実際に使用可能な文字種を増やす部分の変更は一旦削除かコメントアウトしてもらって、それ以外の変更は一足先に入れるで良いかもしれないわね

使用可能な文字種の追加に関するコードを削除しました。

@syuilo syuilo merged commit 008538a into aiscript-dev:master Sep 20, 2025
4 checks passed
@syuilo
Copy link
Collaborator

syuilo commented Sep 20, 2025

👍

@takejohn takejohn deleted the enhance/889_json5-identifiers branch September 20, 2025 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants