Add format specifier rules #14

yshrkt · 2024-03-18T04:03:00Z

Adds rule to should be '%lld' instead of '%d' as integer format specifier.
Adds rule to should be same number of format specifiers between key and value.

Summary by CodeRabbit

新機能
- 整数形式指定子に %lld を使用するルールを追加しました。
- ローカライズプロジェクトの文字列ファイル内のキーと値のペア間の形式指定子の数を検証するルールを追加しました。
テスト
- 整数形式指定子ルールとキー値形式指定子数ルールを検証する新しいテストメソッドを追加しました。

s2mr

LGTM

s2mr · 2024-05-07T05:53:07Z

/review

s2mr · 2024-05-07T05:54:02Z

/review

github-actions · 2024-05-07T05:54:32Z

PR Analysis

メインテーマ

新しい整数フォーマット指定子ルールとキー値フォーマット指定子カウントルールの追加

PR サマリー

このPRは、EmbeddedRules.swiftに整数フォーマット指定子ルールとキー値フォーマット指定子カウントルールを追加し、新しいファイルIntegerFormatSpecifierRule.swiftとKeyValueFormatSpecifierCountRule.swiftを導入しています。

PR タイプ

Enhancement

PR フィードバック:

一般的なフィードバック

コードの拡張と柔軟性の向上を図るために、新しい整数フォーマット指定子ルールとキー値フォーマット指定子カウントルールの追加は素晴らしい取り組みです。これにより、多言語化プロジェクトの品質管理が向上し、ローカライズされた文字列の一貫性が維持されるでしょう。

コードのフィードバック

EmbeddedRules.swift
- 関連ファイル: Sources/L10nLintFramework/Models/EmbeddedRules.swift
- 提案:
```
+        IntegerFormatSpecifierRule.self,
+        KeyValueFormatSpecifierCountRule.self,
```
  追加されたルールを適用することで、新しいルールが有効になるように defaultEnabledRules にも追加してください。

RulesTests.swift

関連ファイル: Tests/L10nLintFrameworkTests/RulesTests.swift

提案:

+    func testIntegerFormatSpecifierRule() throws {
+        let baseProject = try TestHelper.localizedProjects(fixtureName: "Localizables9")
+            .first(where: { $0.name == "Base" })!
+
+        let violations = try IntegerFormatSpecifierRule().validate(baseProject: baseProject, project: baseProject)
+        XCTAssertEqual(
+            violations.map(\.location.point),
+            [
+                LocationPoint(line: 2, character: 20),
+                LocationPoint(line: 3, character: 22),
+                LocationPoint(line: 4, character: 7)
+            ]
+        )
+    }

+    func testKeyValueFormatSpecifierCountRule() throws {
+        let baseProject = try TestHelper.localizedProjects(fixtureName: "Localizables9")
+            .first(where: { $0.name == "Base" })!
+
+        let violations = try KeyValueFormatSpecifierCountRule().validate(baseProject: baseProject, project: baseProject)
+        XCTAssertEqual(
+            violations.map(\.location.point),
+            [
+                LocationPoint(line: 3, character: 1),
+                LocationPoint(line: 4, character: 1),
+                LocationPoint(line: 8, character: 1),
+                LocationPoint(line: 9, character: 1),
+                LocationPoint(line: 10, character: 1)
+            ]
+        )
+    }

テストケースが整数フォーマット指定子ルールとキー値フォーマット指定子カウントルールを適切にテストするように追加されています。

セキュリティ上の懸念:

no

提案された変更は安全であり、セキュリティ上の懸念はありません。

s2mr · 2024-05-07T06:29:46Z

@coderabbitai review

coderabbitai · 2024-05-07T06:29:56Z

Actions Performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2024-05-07T06:29:58Z

ウォークスルー

このアップデートでは、L10nLintFrameworkに新しいルールが追加され、整数フォーマット指定子の使用とキーと値のペアのフォーマット指定子数の検証が強化されました。テストも強化され、より堅牢なローカライズプロセスをサポートします。

変更点

ファイルパス	変更概要
`.../Models/EmbeddedRules.swift`	`IntegerFormatSpecifierRule` と `KeyValueFormatSpecifierCountRule` をルールリストに追加
`.../Rules/IntegerFormatSpecifierRule.swift`	新しいルール `Integer Format Specifier` を追加、`%lld` の使用を強制
`.../Rules/KeyValueFormatSpecifierCountRule.swift`	新しいルール `KeyValueFormatSpecifierCountRule` を追加、キーと値のフォーマット指定子数を検証
`.../Tests/Resources/Fixtures/Localizables9/Base.lproj/Localizable.strings`	データタイプと順序の異なるプレースホルダを持つキー値ペアを追加
`.../Tests/RulesTests.swift`	`IntegerFormatSpecifierRule` と `KeyValueFormatSpecifierCountRule` のテストメソッドを追加

🐰🎉
コードの森を駆け巡る小さなウサギよ
新たなルールを背に、バグを追い払う
型安全の光に導かれ
文字列の海を渡る冒険者
今日も一歩、前へ

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 4

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e991076 and b16f5ce.

Files selected for processing (5)

Sources/L10nLintFramework/Models/EmbeddedRules.swift (2 hunks)
Sources/L10nLintFramework/Rules/IntegerFormatSpecifierRule.swift (1 hunks)
Sources/L10nLintFramework/Rules/KeyValueFormatSpecifierCountRule.swift (1 hunks)
Tests/L10nLintFrameworkTests/Resources/Fixtures/Localizables9/Base.lproj/Localizable.strings (1 hunks)
Tests/L10nLintFrameworkTests/RulesTests.swift (1 hunks)

Additional comments not posted (4)

Sources/L10nLintFramework/Models/EmbeddedRules.swift (1)

13-14: 新しいルールが正しくリストに追加されています。

Also applies to: 28-29

Sources/L10nLintFramework/Rules/KeyValueFormatSpecifierCountRule.swift (1)

9-44: キーと値の間でフォーマット指定子の数が一致するかどうかを検証するロジックが適切に実装されています。

Tests/L10nLintFrameworkTests/RulesTests.swift (2)

170-182: IntegerFormatSpecifierRuleのテストが適切に実装されています。

185-199: KeyValueFormatSpecifierCountRuleのテストが適切に実装されています。

coderabbitai · 2024-05-07T06:33:01Z

Tests/L10nLintFrameworkTests/Resources/Fixtures/Localizables9/Base.lproj/Localizable.strings

@@ -0,0 +1,10 @@
+"key1_%lld" = "value %lld";
+"key2_%d" = "value %d";


%dの使用は%lldに置き換える必要があります。

coderabbitai · 2024-05-07T06:33:01Z

Tests/L10nLintFrameworkTests/Resources/Fixtures/Localizables9/Base.lproj/Localizable.strings

@@ -0,0 +1,10 @@
+"key1_%lld" = "value %lld";
+"key2_%d" = "value %d";
+"key3_%lld" = "value %d";


キーと値のフォーマット指定子の数が一致していません。

coderabbitai · 2024-05-07T06:33:01Z

Tests/L10nLintFrameworkTests/Resources/Fixtures/Localizables9/Base.lproj/Localizable.strings

+"key1_%lld" = "value %lld";
+"key2_%d" = "value %d";
+"key3_%lld" = "value %d";
+"key4_%d" = "value %lld";


キーと値のフォーマット指定子の数が一致していません。

coderabbitai · 2024-05-07T06:33:01Z

Sources/L10nLintFramework/Rules/IntegerFormatSpecifierRule.swift

+    init() {}
+
+    func validate(baseProject: LocalizedProject, project: LocalizedProject) throws -> [StyleViolation] {
+        let regex = try NSRegularExpression(pattern: #"^.*(?<specifier>%[0-9]*d).*$"#, options: .anchorsMatchLines)


正規表現のパターンが%dのみを検出するように設計されていますが、エスケープされたフォーマット指定子や他のコンテキストで使用されている%dも誤って検出する可能性があります。

s2mr · 2024-05-07T07:36:39Z

/pragent-review

s2mr · 2024-05-07T07:43:18Z

/review

github-actions · 2024-05-07T07:44:43Z

PR Review 🔍

⏱️ Estimated effort to review [1-5]	3, このPRは新しいルールの追加とそれに関連するテストの追加を含んでおり、コードの理解とレビューにはある程度の時間が必要です。
🧪 Relevant tests	Yes
⚡ Possible issues	No
🔒 Security concerns	No

Code feedback:

relevant file	Sources/L10nLintFramework/Rules/IntegerFormatSpecifierRule.swift
suggestion	正規表現のパターンについて、`%[0-9]*d` は `%d` だけでなく、`%2d` のような形式もマッチしてしまいます。これが意図した動作でない場合は、パターンを修正することを検討してください。 [important]
relevant line	let regex = try NSRegularExpression(pattern: #"^.(?%[0-9]d).*$"#, options: .anchorsMatchLines)

relevant file	Sources/L10nLintFramework/Rules/KeyValueFormatSpecifierCountRule.swift
suggestion	`keyFormatSpecifierCounts` と `valueFormatSpecifierCounts` の比較で、キーのセットが異なる場合と値が異なる場合の両方で `StyleViolation` を生成していますが、これにより同じ位置で複数の警告が生成される可能性があります。これを避けるために、一つの `StyleViolation` で複数の問題を報告するか、異なる位置に警告を出すように調整することを検討してください。 [important]
relevant line	if Set(keyFormatSpecifierCounts.keys) != Set(valueFormatSpecifierCounts.keys) {

relevant file	Sources/L10nLintFramework/Rules/KeyValueFormatSpecifierCountRule.swift
suggestion	`formatSpecifierRegex` のパターンが非常に複雑で、理解しにくい可能性があります。よりシンプルな方法やコメントを追加して、この正規表現の意図を明確にすることを検討してください。 [medium]
relevant line	let formatSpecifierRegex = try NSRegularExpression(pattern: #"%[0-9]*(?([@cCsSp]\|(([hlqztj]\|ll)?[dDuUxXoO])\|(L?[aAeEfFg])))"#)

yshrkt added 5 commits March 18, 2024 12:48

adds integer format specifier rule

590ef2e

adds key value format specifier count rule

db9cfe2

updates rule tests

260d4ba

updates embedded rules

1c0f64f

fixes regex for detecting format specifiers

b16f5ce

s2mr approved these changes Mar 21, 2024

View reviewed changes

s2mr merged commit 6fa4d9a into s2mr:main Mar 21, 2024

coderabbitai bot reviewed May 7, 2024

View reviewed changes

github-actions bot added the Review effort [1-5]: 3 label May 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add format specifier rules #14

Add format specifier rules #14

yshrkt commented Mar 18, 2024 •

edited by coderabbitai bot

Loading

s2mr left a comment

s2mr commented May 7, 2024

s2mr commented May 7, 2024

github-actions bot commented May 7, 2024

s2mr commented May 7, 2024

coderabbitai bot commented May 7, 2024

coderabbitai bot commented May 7, 2024 •

edited

Loading

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

coderabbitai bot left a comment

coderabbitai bot May 7, 2024

coderabbitai bot May 7, 2024

coderabbitai bot May 7, 2024

coderabbitai bot May 7, 2024

s2mr commented May 7, 2024

s2mr commented May 7, 2024

github-actions bot commented May 7, 2024

		@@ -0,0 +1,10 @@
		"key1_%lld" = "value %lld";
		"key2_%d" = "value %d";

Add format specifier rules #14

Add format specifier rules #14

Conversation

yshrkt commented Mar 18, 2024 • edited by coderabbitai bot Loading

Summary by CodeRabbit

s2mr left a comment

Choose a reason for hiding this comment

s2mr commented May 7, 2024

s2mr commented May 7, 2024

github-actions bot commented May 7, 2024

PR Analysis

メインテーマ

PR サマリー

PR タイプ

PR フィードバック:

一般的なフィードバック

コードのフィードバック

セキュリティ上の懸念:

s2mr commented May 7, 2024

coderabbitai bot commented May 7, 2024

coderabbitai bot commented May 7, 2024 • edited Loading

ウォークスルー

変更点

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot May 7, 2024

Choose a reason for hiding this comment

coderabbitai bot May 7, 2024

Choose a reason for hiding this comment

coderabbitai bot May 7, 2024

Choose a reason for hiding this comment

coderabbitai bot May 7, 2024

Choose a reason for hiding this comment

s2mr commented May 7, 2024

s2mr commented May 7, 2024

github-actions bot commented May 7, 2024

PR Review 🔍

yshrkt commented Mar 18, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented May 7, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)