-
Notifications
You must be signed in to change notification settings - Fork 5
Preview/prismalint #36
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
Conversation
WalkthroughA new Prisma schema file and a Prisma linting configuration file have been introduced. The schema defines four models with explicit mappings and relations, while the linting configuration enforces naming, ordering, and indexing conventions for Prisma models and fields. Changes
Poem
Impact AnalysisIntroduction of Prisma schema linting rules for naming and structure consistencyℹ️ Low 🔄 Impacts behavior A new linting configuration enforces naming conventions, field ordering, and structural rules for Prisma schema files. Developers will now receive errors or warnings if their Prisma models or fields do not comply with these standards, promoting consistency but potentially requiring schema changes to pass linting. Test Prisma schema files with both compliant and non-compliant naming and structure to verify that linting errors and warnings are triggered as expected. Validate that developer workflows are not unduly disrupted. 🔍 Related Files
Addition of initial Prisma schema with example models and mappingsℹ️ Low 🔄 Impacts behavior A new Prisma schema file defines several example models, relations, and mappings to database tables and columns. This enables developers to generate a Prisma client and interact with a sample PostgreSQL database structure. Generate the Prisma client and test basic CRUD operations for each model. Verify that relations, ignored fields, and custom mappings behave as expected in the generated client and database. 🔍 Related Files
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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: 4
🧹 Nitpick comments (2)
.prismalintrc.json (1)
1-36
: Consider pinning the prismalint versionThe rule set is fairly new and still evolving. Pinning the CLI version in
package.json
(or adevDependencies
block) will ensure future minor releases don’t break your CI unexpectedly.example.prisma (1)
26-30
:UserRoleFoo
mapping looks inconsistentThe model name is singular (✅) but the mapped table name is
unexpected_snake_case
, which does not reflect the model’s semantics.
Consider renaming the model to something meaningful or adjusting the@@map
to match the business domain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.prismalintrc.json
(1 hunks)example.prisma
(1 hunks)
🔇 Additional comments (2)
.prismalintrc.json (1)
9-14
: Use correct spread token forfield-order
prismalint’s schema defines the rest operator as
"$rest"
, not"..."
. Using"..."
will be ignored and your field ordering won’t be enforced. Please verify in your installed prismalint package (for example, search for"$rest"
in its schema) and then update your config:"field-order": [ "error", { - "order": ["tenantId", "..."] + "order": ["tenantId", "$rest"] } ],example.prisma (1)
20-24
: No issues detected withTenant
Naming, mapping and minimal field set look good.
"require-field-index": [ | ||
"error", | ||
{ | ||
"forAllRelations": true, | ||
"forNames": ["tenantId"] | ||
} | ||
] |
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.
🛠️ Refactor suggestion
Option name forNames
looks suspicious
Up-stream docs use forFieldNames
; the short form forNames
is not mentioned.
If this key is wrong, the entire require-field-index
rule will be ignored and the schema violations you are trying to catch (e.g. missing @@index([tenantId])
) will slip through.
- "forNames": ["tenantId"]
+ "forFieldNames": ["tenantId"]
📝 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.
"require-field-index": [ | |
"error", | |
{ | |
"forAllRelations": true, | |
"forNames": ["tenantId"] | |
} | |
] | |
"require-field-index": [ | |
"error", | |
{ | |
"forAllRelations": true, | |
"forFieldNames": ["tenantId"] | |
} | |
] |
🤖 Prompt for AI Agents
In the .prismalintrc.json file between lines 28 and 34, the option name
"forNames" used in the "require-field-index" rule is incorrect and should be
changed to "forFieldNames" to match the upstream documentation. Update the key
from "forNames" to "forFieldNames" to ensure the rule is properly recognized and
enforced, preventing schema violations from being missed.
datasource db { | ||
provider = "postgresql" | ||
url = "fake-url" | ||
} |
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.
🛠️ Refactor suggestion
Hard-coding the connection string makes local & CI setup brittle
Replace the literal "fake-url"
with an env("DATABASE_URL")
reference so developers and CI can supply their own credentials without touching the schema.
- url = "fake-url"
+ url = env("DATABASE_URL")
📝 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.
datasource db { | |
provider = "postgresql" | |
url = "fake-url" | |
} | |
datasource db { | |
provider = "postgresql" | |
url = env("DATABASE_URL") | |
} |
🤖 Prompt for AI Agents
In example.prisma around lines 6 to 9, the datasource url is hard-coded as
"fake-url", which makes local and CI setups brittle. Replace the literal string
with env("DATABASE_URL") to load the connection string from an environment
variable, allowing developers and CI pipelines to supply their own database
credentials without modifying the schema file.
model UserRole { | ||
id String @id | ||
userId String @map(name: "userid") | ||
// No mapping. | ||
} |
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.
🛠️ Refactor suggestion
UserRole
lacks a relation and required index
userId
is clearly intended to reference User
, yet no @relation
is declared and no index is present.
This will also violate the require-field-index
rule (forAllRelations: true
).
model UserRole {
id String @id
- userId String @map(name: "userid")
- // No mapping.
+ userId String @map(name: "userid")
+ user User @relation(fields: [userId], references: [id])
+
+ @@index([userId])
}
📝 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.
model UserRole { | |
id String @id | |
userId String @map(name: "userid") | |
// No mapping. | |
} | |
model UserRole { | |
id String @id | |
userId String @map(name: "userid") | |
user User @relation(fields: [userId], references: [id]) | |
@@index([userId]) | |
} |
🤖 Prompt for AI Agents
In example.prisma around lines 31 to 35, the UserRole model's userId field lacks
a @relation attribute linking it to the User model and does not have an index,
which is required for foreign key fields. Add a proper @relation directive to
userId referencing the User model and create an index on userId to comply with
the require-field-index rule and establish the relationship correctly.
model Users { | ||
id String @id | ||
emailAddress String | ||
tenantId String | ||
removeMe String @ignore | ||
tenant Tenant @relation(fields: [tenantId], references: [id]) | ||
@@map(name: "users") | ||
} |
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.
🛠️ Refactor suggestion
Users
model violates multiple lint rules
- Model name is plural ⇒ fails
model-name-grammatical-number
. tenantId
is not the first field ⇒ failsfield-order
.tenantId
is missing an index ⇒ failsrequire-field-index
.- Required field
removeMe
is ignored ⇒ failsforbid-required-ignored-field
.
-model Users {
+model User {
id String @id
- emailAddress String
- tenantId String
- removeMe String @ignore
+ tenantId String
+ emailAddress String
+ // removeMe was required but ignored. Either make it optional or drop it entirely.
+ // removeMe String? @ignore
+
+ @@index([tenantId])
Renaming the model will require cascading changes in your codebase & migrations, but it brings the schema in line with the configured rules.
📝 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.
model Users { | |
id String @id | |
emailAddress String | |
tenantId String | |
removeMe String @ignore | |
tenant Tenant @relation(fields: [tenantId], references: [id]) | |
@@map(name: "users") | |
} | |
model User { | |
id String @id | |
tenantId String | |
emailAddress String | |
// removeMe was required but ignored. Either make it optional or drop it entirely. | |
// removeMe String? @ignore | |
@@index([tenantId]) | |
tenant Tenant @relation(fields: [tenantId], references: [id]) | |
@@map(name: "users") | |
} |
🤖 Prompt for AI Agents
In example.prisma around lines 11 to 18, the Users model violates lint rules:
rename the model to singular form (User), move tenantId to be the first field,
add an index on tenantId, and either remove the required field removeMe or make
it optional if it must be ignored. These changes align the schema with lint
rules but will require updates in your codebase and migrations.
Summary by CodeRabbit