-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
unkownutil周りのライブラリを上げて、型判定周りを整理 #93
Conversation
WalkthroughThe updates across various files enhance type safety and validation mechanisms by transitioning from utility functions to assertion-based checks. This shift from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Source
participant Validator
User->>App: Initiates action
App->>Source: Call onInit()
Source->>Validator: Validate path
Validator-->>Source: Assert path is string
Source-->>App: Proceed with valid path
App-->>User: Action completed
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configuration File (
|
一度 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (1)
denops/@ddc-sources/stamp.ts (1)
19-19
: Ensure proper error handling foronInit
function callsThe
onInit
function calls in the following files do not handle the potential assertion error correctly. Please add appropriate error handling to manage assertion failures:
denops/@ddc-sources/stamp.ts
denops/@ddu-sources/channel_rec.ts
denops/@ddu-sources/channel.ts
Analysis chain
LGTM! But verify the function usage in the codebase.
The code changes are approved.
However, ensure that all function calls to
onInit
handle the potential assertion error correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `onInit` handle the potential assertion error correctly. # Test: Search for the function usage. Expect: Proper error handling in the calling code. rg --type python -A 5 $'onInit'Length of output: 61
Script:
#!/bin/bash # Description: Verify all function calls to `onInit` handle the potential assertion error correctly. # Test: Search for the function usage. Expect: Proper error handling in the calling code. rg --type ts -A 5 $'onInit'Length of output: 1352
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- denops/@ddc-sources/stamp.ts (2 hunks)
- denops/@ddu-kinds/channel.ts (3 hunks)
- denops/@ddu-sources/channel.ts (2 hunks)
- denops/@ddu-sources/channel_rec.ts (2 hunks)
- denops/traqvim/action.ts (8 hunks)
- denops/traqvim/deps.ts (1 hunks)
- denops/traqvim/main.ts (16 hunks)
- denops/traqvim/type_check.ts (1 hunks)
Additional comments not posted (43)
denops/@ddc-sources/stamp.ts (1)
2-6
: LGTM!The import statements have been updated correctly to use the
assert
andis
utilities from the new version ofunknownutil
.denops/traqvim/deps.ts (2)
4-4
: LGTM!The export statements have been updated correctly to use the
assert
andis
utilities from the new version ofunknownutil
.
5-5
: LGTM!The export of
Predicate
has been added correctly to enhance type safety.denops/traqvim/type_check.ts (4)
4-10
: LGTM!The predicate
isMessageStamp
is well-defined and ensures type safety fortraq.MessageStamp
.
12-19
: LGTM!The predicate
isUserAccountState
is well-defined and ensures type safety fortraq.UserAccountState
.
21-29
: LGTM!The predicate
isUser
is well-defined and ensures type safety fortraq.User
.
31-51
: LGTM!The predicate
isMessage
is well-defined and ensures type safety forMessage
. The nested usage ofisMessage
in thequote
property is a good approach for recursive type checking.denops/@ddu-sources/channel_rec.ts (1)
22-22
: LGTM!The change to use
assert
withis.String
improves type safety and error handling.denops/@ddu-kinds/channel.ts (2)
36-36
: LGTM!The change to use
assert
withis.Number
improves type safety and error handling.
77-77
: LGTM!The change to use
assert
withis.ArrayOf(is.String)
improves type safety and error handling.denops/@ddu-sources/channel.ts (1)
21-21
: LGTM!The change to use
assert
withis.String
improves type safety and error handling.denops/traqvim/action.ts (7)
60-60
: LGTM! Improved type safety with assert.The use of
assert(timeline, is.ArrayOf(isMessage))
enhances type safety and ensures that the timeline is an array ofMessage
objects.
83-83
: LGTM! Improved type safety with assert.The use of
assert(timeline, is.ArrayOf(isMessage))
enhances type safety and ensures that the timeline is an array ofMessage
objects.
107-107
: LGTM! Improved type safety with assert.The use of
assert(timeline, is.ArrayOf(isMessage))
enhances type safety and ensures that the timeline is an array ofMessage
objects.
132-132
: LGTM! Improved type safety with assert.The use of
assert(timeline, is.ArrayOf(isMessage))
enhances type safety and ensures that the timeline is an array ofMessage
objects.
213-213
: LGTM! Improved type safety with assert.The use of
assert(timeline, is.ArrayOf(isMessage))
enhances type safety and ensures that the timeline is an array ofMessage
objects.
243-243
: LGTM! Improved type safety with assert.The use of
assert(timeline, is.ArrayOf(isMessage))
enhances type safety and ensures that the timeline is an array ofMessage
objects.
Line range hint
1-12
:
LGTM! Updated imports for type safety.The additions of
assert
,is
, andisMessage
are consistent with the updated validation methods and improve type safety.denops/traqvim/main.ts (25)
29-29
: LGTM! Improved type safety with assert.The use of
assert(path, is.String)
enhances type safety and ensures that thepath
is a string.
57-57
: LGTM! Improved type safety with assert.The use of
assert(choice, is.Number)
enhances type safety and ensures that thechoice
is a number.
82-82
: LGTM! Improved type safety with assert.The use of
assert(limit, is.Number)
enhances type safety and ensures that thelimit
is a number.
94-94
: LGTM! Improved type safety with assert.The use of
assert(channelPath, is.String)
enhances type safety and ensures that thechannelPath
is a string.
102-102
: LGTM! Improved type safety with assert.The use of
assert(channelID, is.String)
enhances type safety and ensures that thechannelID
is a string.
104-104
: LGTM! Improved type safety with assert.The use of
assert(limit, is.Number)
enhances type safety and ensures that thelimit
is a number.
121-122
: LGTM! Improved type safety with assert.The use of
assert(bufNum, is.Number)
andassert(bufName, is.String)
enhances type safety and ensures thatbufNum
is a number andbufName
is a string.
132-132
: LGTM! Improved type safety with assert.The use of
assert(channelID, is.String)
enhances type safety and ensures that thechannelID
is a string.
134-134
: LGTM! Improved type safety with assert.The use of
assert(limit, is.Number)
enhances type safety and ensures that thelimit
is a number.
147-148
: LGTM! Improved type safety with assert.The use of
assert(bufNum, is.Number)
andassert(bufName, is.String)
enhances type safety and ensures thatbufNum
is a number andbufName
is a string.
152-152
: LGTM! Improved type safety with assert.The use of
assert(timeline, is.ArrayOf(isMessage))
enhances type safety and ensures that thetimeline
is an array ofMessage
objects.
155-155
: LGTM! Improved type safety with assert.The use of
assert(channelID, is.String)
enhances type safety and ensures that thechannelID
is a string.
157-157
: LGTM! Improved type safety with assert.The use of
assert(limit, is.Number)
enhances type safety and ensures that thelimit
is a number.
184-185
: LGTM! Improved type safety with assert.The use of
assert(bufNum, is.Number)
andassert(bufName, is.String)
enhances type safety and ensures thatbufNum
is a number andbufName
is a string.
189-189
: LGTM! Improved type safety with assert.The use of
assert(timeline, is.ArrayOf(isMessage))
enhances type safety and ensures that thetimeline
is an array ofMessage
objects.
192-192
: LGTM! Improved type safety with assert.The use of
assert(channelID, is.String)
enhances type safety and ensures that thechannelID
is a string.
194-194
: LGTM! Improved type safety with assert.The use of
assert(limit, is.Number)
enhances type safety and ensures that thelimit
is a number.
219-220
: LGTM! Improved type safety with assert.The use of
assert(bufNum, is.Number)
andassert(bufName, is.String)
enhances type safety and ensures thatbufNum
is a number andbufName
is a string.
222-222
: LGTM! Improved type safety with assert.The use of
assert(channelID, is.String)
enhances type safety and ensures that thechannelID
is a string.
252-252
: LGTM! Improved type safety with assert.The use of
assert(bufNum, is.Number)
enhances type safety and ensures thatbufNum
is a number.
254-254
: LGTM! Improved type safety with assert.The use of
assert(channelID, is.String)
enhances type safety and ensures that thechannelID
is a string.
270-270
: LGTM! Improved type safety with assert.The use of
assert(bufNum, is.Number)
enhances type safety and ensures thatbufNum
is a number.
278-278
: LGTM! Improved type safety with assert.The use of
assert(choice, is.Number)
enhances type safety and ensures that thechoice
is a number.
300-300
: LGTM! Improved type safety with assert.The use of
assert(bufNum, is.Number)
enhances type safety and ensures thatbufNum
is a number.
311-311
: LGTM! Improved type safety with assert.The use of
assert(messageBufNum, is.Number)
enhances type safety and ensures that themessageBufNum
is a number.
assert, | ||
dduVim, | ||
dduVimSource, | ||
Denops, | ||
ensureString, | ||
is, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused imports.
The dduVim
, dduVimSource
, and Denops
imports are not used in this file.
-import {
- assert,
- dduVim,
- dduVimSource,
- Denops,
- is,
- vars,
-} from "../traqvim/deps.ts";
+import { assert, is, vars } from "../traqvim/deps.ts";
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
assert, | |
dduVim, | |
dduVimSource, | |
Denops, | |
ensureString, | |
is, | |
import { assert, is, vars } from "../traqvim/deps.ts"; |
denops/@ddu-kinds/channel.ts
Outdated
ensureNumber, | ||
vars, | ||
} from "../traqvim/deps.ts"; | ||
import { assert, dduVim, Denops, is, vars } from "../traqvim/deps.ts"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused imports.
The vars
import is not used in this file.
-import { assert, dduVim, Denops, is, vars } from "../traqvim/deps.ts";
+import { assert, dduVim, Denops, is } from "../traqvim/deps.ts";
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { assert, dduVim, Denops, is, vars } from "../traqvim/deps.ts"; | |
import { assert, dduVim, Denops, is } from "../traqvim/deps.ts"; |
assert, | ||
dduVim, | ||
dduVimSource, | ||
Denops, | ||
ensureString, | ||
helper, | ||
is, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused imports.
The dduVim
, dduVimSource
, Denops
, and helper
imports are not used in this file.
-import {
- assert,
- dduVim,
- dduVimSource,
- Denops,
- helper,
- is,
- vars,
-} from "../traqvim/deps.ts";
+import { assert, is, vars } from "../traqvim/deps.ts";
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
assert, | |
dduVim, | |
dduVimSource, | |
Denops, | |
ensureString, | |
helper, | |
is, | |
import { assert, is, vars } from "../traqvim/deps.ts"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
denops/traqvim/model.ts (1)
136-136
: Inconsistent validation ofgetMeInfo
resultsThe function
getMeInfo
is used in two places withindenops/traqvim/model.ts
. While the second usage includes validation withensure
, the first usage does not. To maintain consistency and ensure type safety, consider addingensure
validation in the first usage as well.
- First usage:
denops/traqvim/model.ts
(lines followingconst me = await getMeInfo();
)- Second usage:
denops/traqvim/model.ts
(lines followingconst me = await getMeInfo();
withensure
validation)Analysis chain
LGTM! But verify the function usage in the codebase.
The use of
ensure
withis.String
enhances type safety and robustness.However, ensure that all function calls to
getMeInfo
are properly validated.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `getMeInfo` are properly validated. # Test: Search for the function usage. Expect: Proper validation with `ensure`. rg --type python -A 5 $'getMeInfo'Length of output: 64
Script:
#!/bin/bash # Description: Verify all function calls to `getMeInfo` are properly validated. # Test: Search for the function usage. Expect: Proper validation with `ensure`. rg --type ts -A 5 'getMeInfo'Length of output: 890
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- denops/@ddc-sources/stamp.ts (2 hunks)
- denops/@ddu-columns/channel.ts (2 hunks)
- denops/@ddu-kinds/channel.ts (4 hunks)
- denops/traqvim/deps.ts (1 hunks)
- denops/traqvim/main.ts (16 hunks)
- denops/traqvim/model.ts (2 hunks)
- denops/traqvim/type_check.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- denops/traqvim/type_check.ts
Files skipped from review as they are similar to previous changes (3)
- denops/@ddc-sources/stamp.ts
- denops/traqvim/deps.ts
- denops/traqvim/main.ts
Additional comments not posted (4)
denops/@ddu-columns/channel.ts (1)
39-39
: LGTM! But verify the function usage in the codebase.The use of
ensure
withis.Number
enhances type safety and robustness.However, ensure that all function calls to
fn.strwidth
are properly validated.Verification successful
Let's correct the file type and re-run the search for
fn.strwidth
function calls in TypeScript files.
Let's correct the file type by specifying the
.ts
extension and re-run the search forfn.strwidth
function calls.
Verified: The function usage is properly validated.
The
fn.strwidth
function is used indenops/@ddu-columns/channel.ts
and is correctly validated withensure
andis.Number
.
denops/@ddu-columns/channel.ts
, line 39:const width = ensure(await fn.strwidth(args.denops, text), is.Number);
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `fn.strwidth` are properly validated. # Test: Search for the function usage. Expect: Proper validation with `ensure`. rg --type python -A 5 $'fn.strwidth'Length of output: 66
Script:
#!/bin/bash # Description: Verify all function calls to `fn.strwidth` are properly validated. # Test: Search for the function usage. Expect: Proper validation with `ensure`. rg --type typescript -A 5 $'fn.strwidth'Length of output: 74
Script:
#!/bin/bash # Description: Verify all function calls to `fn.strwidth` are properly validated. # Test: Search for the function usage. Expect: Proper validation with `ensure`. rg --glob '*.ts' -A 5 $'fn.strwidth'Length of output: 434
denops/@ddu-kinds/channel.ts (3)
2-2
: LGTM!The inclusion of
assert
suggests a shift towards more explicit type assertions, enhancing type safety.
21-22
: LGTM!The use of
is.ObjectOf
withis.String
ensures thatid
is a string, enhancing type safety.
52-52
: LGTM! But verify the function usage in the codebase.The use of
assert
withis.Number
enhances type safety and error handling.However, ensure that all function calls to
vars.globals.get
are properly validated.Verification successful
Verified: All instances of
vars.globals.get
are properly validated withassert
.The use of
assert
withis.String
andis.Number
ensures that the values retrieved byvars.globals.get
are properly validated, enhancing type safety and error handling.
denops/@ddc-sources/stamp.ts
denops/traqvim/main.ts
denops/@ddu-kinds/channel.ts
denops/@ddu-sources/channel.ts
denops/@ddu-sources/channel_rec.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `vars.globals.get` are properly validated. # Test: Search for the function usage. Expect: Proper validation with `assert`. rg --type python -A 5 $'vars.globals.get'Length of output: 71
Script:
#!/bin/bash # Description: Verify all function calls to `vars.globals.get` are properly validated. # Test: Search for the function usage. Expect: Proper validation with `assert`. rg --type typescript -A 5 'vars.globals.get'Length of output: 79
Script:
#!/bin/bash # Description: Verify all function calls to `vars.globals.get` are properly validated. # Test: Search for the function usage. Expect: Proper validation with `assert`. rg --type ts -A 5 'vars.globals.get'Length of output: 3851
Predicateだとメタデータの情報が落ちるらしいから、折をみて |
#60
↑これに関連する
Summary by CodeRabbit
New Features
Bug Fixes
Documentation