Conversation
We had this set to warning to allow our legacy code to pass lint, but this has result in more uses of explicit any being added to the codebase. The approach here is to add comments to allow the existing cases but ban all new cases.
WalkthroughThis change set introduces a series of comments and lint suppression directives across the codebase to address the use of the Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/post/post.entity.ts (1)
155-157: Properly documented legacyanyusage in handleDeleted method.The added comments provide appropriate context for this type cast while suppressing the linting warning. For future improvement, consider refactoring this method to avoid type casting.
In the future, you might want to consider refactoring this to use a better typing approach instead of casting to
any. For example, you could define a mutable version of the interface with optional properties that can be modified.src/post/post.entity.unit.test.ts (1)
208-210: Consistently documentedanyusage across test cases.The pattern of adding TODO comments and lint directives has been consistently applied to all occurrences of
anycasting in the test file.For future refactoring, consider creating a test utility function that creates a properly typed mock post with an ID, which would eliminate the need for multiple
anycasts throughout these tests.Also applies to: 225-227, 242-244, 259-261, 277-279
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
biome.json(1 hunks)cedar/query-runner/src/app.ts(2 hunks)src/activity-handlers/create.handler.ts(1 hunks)src/app.ts(1 hunks)src/core/events.ts(1 hunks)src/db.ts(1 hunks)src/dispatchers.ts(2 hunks)src/dispatchers.unit.test.ts(6 hunks)src/helpers/activitypub/activity.ts(1 hunks)src/helpers/user.ts(1 hunks)src/helpers/user.unit.test.ts(1 hunks)src/http/api/helpers/account.ts(1 hunks)src/http/api/helpers/account.unit.test.ts(3 hunks)src/http/api/views/account.follows.view.ts(1 hunks)src/kv-helpers.ts(1 hunks)src/lookup-helpers.unit.test.ts(5 hunks)src/post/post.entity.ts(1 hunks)src/post/post.entity.unit.test.ts(6 hunks)src/post/post.repository.knex.ts(3 hunks)src/post/post.service.ts(2 hunks)src/post/post.service.unit.test.ts(2 hunks)
🔇 Additional comments (34)
src/helpers/user.unit.test.ts (1)
63-65: Legacyanyusage is properly annotated and suppressed.The addition of the TODO and biome-ignore comments above the legacy
anycast is appropriate and aligns with the project's new linting policy. No further action needed.biome.json (1)
31-32: Lint configuration update aligns with new type safety policy.Removing global "noExplicitAny" and "noImplicitAnyLet" rules is correct, given the new approach of inline suppression for legacy code. This supports incremental migration to stricter typing.
src/activity-handlers/create.handler.ts (1)
47-49: Legacyanyusage is clearly annotated and suppressed.The TODO and biome-ignore comments above the legacy
anyusage for database retrieval are appropriate and support the project's linting policy. No further action required.src/helpers/user.ts (1)
147-150: Legacyanyusage is properly annotated and suppressed.The TODO and biome-ignore comments above the legacy
anyusage for keypair retrieval are appropriate and align with the project's linting policy. No further action needed.src/core/events.ts (1)
4-6: Legacyanyusage is properly annotated and suppressed.The TODO and biome-ignore comments above the legacy
anyusage for event arguments are appropriate and align with the project's linting policy. No further action needed.src/kv-helpers.ts (1)
13-15: Appropriate annotation and lint suppression for legacyanyusage.The addition of a TODO and biome-ignore directive above the explicit
anyparameter is a good practice for managing legacy code. This makes technical debt visible and prevents new untracked uses ofany.src/app.ts (1)
300-302: Appropriate annotation and lint suppression for legacyanyusage.The TODO and biome-ignore directive above the explicit
anycast make the technical debt visible and prevent new untracked uses ofany. This is a good interim solution for legacy code.cedar/query-runner/src/app.ts (1)
46-48: Appropriate annotation and lint suppression for legacyanyusage.The TODO and biome-ignore directives above the explicit
anyparameters in both functions make the technical debt visible and prevent new untracked uses ofany. This is a good interim solution for legacy code.Also applies to: 56-58
src/lookup-helpers.unit.test.ts (1)
39-41: Appropriate annotation and lint suppression for legacyanyusage in tests.The TODO and biome-ignore directives above the explicit
anycasts in test calls make the technical debt visible and prevent new untracked uses ofany. This is a good interim solution for legacy code in tests.Also applies to: 59-61, 84-86, 99-101, 129-131
src/http/api/views/account.follows.view.ts (1)
198-200: Appropriate annotation and lint suppression for legacyanyusage.The TODO and biome-ignore directive above the explicit
anycast make the technical debt visible and prevent new untracked uses ofany. This is a good interim solution for legacy code.src/db.ts (1)
28-30: Properly documented legacyanyusage.The added comments appropriately identify this index signature as legacy code that requires future typing improvements, while allowing the current code to function.
src/helpers/activitypub/activity.ts (2)
20-22: Properly documented legacyanyusage in ActivityObject interface.The added comments clearly indicate this is technical debt to be addressed in the future while suppressing linting warnings.
28-30: Properly documented legacyanyusage in Activity interface.The comments follow the consistent pattern established across the codebase for handling legacy
anytypes.src/post/post.entity.unit.test.ts (1)
193-195: Properly documentedanyusage in test code.The added comments help identify this test-specific type casting as a technical debt item to be addressed later.
src/http/api/helpers/account.ts (1)
86-88: Properly documented use ofanytype.The added TODO comment and biome-ignore directive appropriately document this legacy use of
anytype while suppressing the linter warning. This approach aligns with the PR's objective of preventing new uses ofanywhile preserving existing ones.src/dispatchers.unit.test.ts (6)
31-33: Appropriate annotation for test-specificanyusage.The added comments properly document this test-specific use of
anywith RequestContext and align with the PR's goal of flagging legacyanyusage for future cleanup.
47-49: Properly documented test data withanytype.Good documentation of the Record<string, any> test data structure with appropriate TODO and lint suppression directive.
88-90: Consistently marked legacyanyusage in test context.The added comments maintain consistency with other similar annotations in this file for RequestContext casting.
156-158: Properly documented test data withanytype.Another instance of Record<string, any> being appropriately documented with TODO and lint suppression directive.
273-275: Consistently marked legacyanyusage in test context.The added comments maintain consistency with other similar annotations in this file for RequestContext casting.
341-343: Properly documented use ofanyin nodeInfoDispatcher test.The added comments appropriately document this test-specific use of
anywith RequestContext and align with the PR's goal.src/http/api/helpers/account.unit.test.ts (5)
237-239: Properly documented use ofanyin test context.The added comments appropriately document this test-specific use of Context and align with the PR's goal of marking legacy
anyusage for future cleanup.
272-274: Consistently documented use ofanytype in test context.The comments maintain consistency with other annotations for Context cases throughout the tests.
276-278: Properly documented directanycast in test.Good annotation for the direct cast to
anyfor the accountService mock in the test context.
290-292: Consistently documented use ofanyin test context.The comments maintain consistency with other annotations for Context cases throughout the tests.
294-296: Consistently documented directanycast in test.Maintains consistency with the previous annotation for the accountService mock, properly documenting direct use of
anyin test context.src/post/post.service.unit.test.ts (2)
100-102: Properly documentedanycast for accessing private method.The added comments appropriately document this test-specific case where
anyis used to access a private method during testing. This follows the PR's approach of marking legacyanyusage while allowing tests to continue functioning.
164-166: Consistently documentedanycast for accessing private method.Maintains consistency with the previous annotation for accessing the private mapActivityToPostDTO method, properly documenting use of
anyin test context.src/post/post.repository.knex.ts (2)
105-117: Appropriate suppression of 'any' type with descriptive commentsThe added TODO comment and biome-ignore directive properly document the legacy use of
anywhile allowing the codebase to function. This follows the PR's objective of banning new uses of explicitanywhile accommodating existing instances.For future improvement, you could consider using a more specific type like:
type PostAttachment = { url: string; [key: string]: unknown; };
444-448: Appropriate use of comment and lint suppression for property mutationThe TODO comment and lint directive are correctly added to acknowledge and temporarily allow this use of
anyfor property mutation.For a future refactor, consider creating a setter method on the Post class to handle ID assignment, which would eliminate the need for type casting:
// In Post class setId(id: number): void { this.id = id; } // Then here post.setId(id);src/post/post.service.ts (2)
151-153: Properly documented legacy 'any' type in method signatureThe added comments correctly highlight this as an area needing type cleanup while allowing the codebase to function with the existing code.
In a future refactor, consider creating a proper interface for the Activity structure:
interface ActivityWithObject { object: { id: string; type: string; name?: string; preview?: { content?: string }; content?: string; /* other properties */ }; actor: { id: string; preferredUsername?: string; name?: string; /* other properties */ }; type: string; /* other properties */ }
661-666: Appropriate documentation of 'any' type cast from toJsonLd resultThe comment and biome-ignore directive correctly identify another instance of legacy
anyusage that needs future cleanup.For a future improvement, consider working with the library maintainers to enhance the typing of the
toJsonLd()method's return value, or create a proper type definition for the expected structure.src/dispatchers.ts (2)
379-383: Well-documented suppression of 'any' for database retrievalThe added comments appropriately document this legacy use of
anyin the database retrieval code. This follows the PR objectives of marking legacy code for future improvement while keeping the codebase functional.For future improvement, consider creating a type for the expected structure of the database entry:
interface ReplyTargetData { attributedTo?: { id: string }; [key: string]: unknown; }
955-966: Properly documented index signatures using 'any'The comments and biome-ignore directives correctly document both instances of index signatures using the
anytype. This follows the PR's approach to addressing legacy code.For future improvement, consider:
interface ObjectWithAttributes { [key: string]: unknown; attributedTo?: string | { [key: string]: unknown }; } interface LikedActivity { object: string | ObjectWithAttributes; [key: string]: unknown; }This would provide better type safety while maintaining flexibility for the diverse data structures.
2 participants
We had this set to warning to allow our legacy code to pass lint, but this has result in more uses of explicit any being added to the codebase. The approach here is to add comments to allow the existing cases but ban all new cases.