Datasource Spans & Completions #3703

Druue · 2023-02-10T15:00:29Z

Changes

Adding datasource completions was necessary as part of this to ensure effective testing. This will require a follow-up PR in language-tools to remove the completions mentioned below from there

Bugs

Completions shouldn't trigger on the closing line with the } on it :: This also existed prior to this PR with the language-tools impl. issue
- Possibly relevant Pest docs. Internal discussion
Property value completions can trigger anywhere after the name, which can be before the = sign :: This is an issue with find_at_position and would probably require changes in the PEST grammar to include having the = as an identifiable piece in the parser for config blocks. issue
Value completions are available after a value (but not immediately like one space after) -- note, this only seems to affect url completions, completions do not show up again on the same line for provider, schemas, and relationMode. issue for the following
This is silly -- XYZ_URL showing up as a completion within the text env but not between v and (
XYZ_URL shows up as a completion anywhere between the left parenthesis and the character right after ) in env(...)

Todo

Remove datasource completions from https://github.com/prisma/language-tools
- This will include sub-completions such as env && "" for url, or XYZ_URL for inside env
update pris.ly links per internal convo
Break-up lift_datasource & lift_generator starting to get a lil unwieldy
Make eldritch language tests more readable to avoid future issues where snapshots change and we don't know what or why
Stub datasource so that we can validate for missing providers / urls prior to having a full datasource (i.e. both exist)?

closes prisma/prisma#17000
Note: This does not entirely fix the issue and is instead a template for future completions additions to engines. This fixes the issue for datasource completions. When, in the future, we add completions to other parts of the schema from engines, this will require similar changes as done here to the PSL.
This is built off the back of the previous pr for adding schemas completions.

Druue · 2023-02-22T10:05:10Z

As a note, further changes in the PSL grammar and parsing were necessary due to issues in the formatter. It ended up being simpler to just add the notion of nesting to the rest of the blocks (as opposed to just the configs -- datasource & generator). So, now they all have some form of xyz_contents, e.g. model_contents which wraps everything between the opening and closing braces of a block.

Druue · 2023-03-03T12:15:22Z

The providers completions, i.e. [mysql, postgresql, sqlite...] are still being emitted from language-tools but the current engines completions aren't suppressing them so I'm inclined to leave those for now.

pimeys

Maybe check the few comments I left?

pimeys · 2023-03-03T14:56:36Z

prisma-fmt/src/get_config.rs

-        let expected = expect![[
-            r#"{"message":"\u001b[1;91merror\u001b[0m: \u001b[1mError validating datasource `thedb`: You must provide a nonempty direct URL\u001b[0m\n  \u001b[1;94m-->\u001b[0m  \u001b[4mschema.prisma:5\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\u001b[1;94m 4 | \u001b[0m                url = env(\"DBURL\")\n\u001b[1;94m 5 | \u001b[0m                directUrl = \u001b[1;91m\"\"\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\nValidation Error Count: 1","error_code":"P1012"}"#
-        ]];
+        let expected = expect![[r#"{"message":"\u001b[1;91merror\u001b[0m: \u001b[1mError validating datasource `thedb`: You must provide a nonempty direct URL\u001b[0m\n  \u001b[1;94m-->\u001b[0m  \u001b[4mschema.prisma:5\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\u001b[1;94m 4 | \u001b[0m                url = env(\"DBURL\")\n\u001b[1;94m 5 | \u001b[0m                \u001b[1;91mdirectUrl = \"\"\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\nValidation Error Count: 1","error_code":"P1012"}"#]];


You don't need to do this now, but by doing the following:

let response = get_config(&requqest.to_string()).unwrap_err(); let response: serde_json::Value = serde_json::from_str(&response).unwrap(); let response = serde_json::to_string_pretty(&response).unwrap();

The resulting string is having indentation, tabs and all that in place, so it's much nicer to read your expectation when it's not a oneliner.

Maybe a morning PR?

Or, even better, deserialize the result, and assert the different parts. Now you can assert the datamodel a bit like we do in the psl tests, and additionally the error code as a separate assert.

prisma-fmt/src/text_document_completion.rs

prisma-fmt/src/text_document_completion/datasource.rs

prisma-fmt/tests/text_document_completion/scenarios/datasource_url_arguments/result.json

psl/psl-core/src/validate/datasource_loader.rs

psl/schema-ast/src/ast.rs

janpio · 2023-03-03T15:38:46Z

prisma-fmt/src/get_config.rs

-        let expected = expect![[
-            r#"{"message":"\u001b[1;91merror\u001b[0m: \u001b[1mEnvironment variable not found: DOES_NOT_EXIST.\u001b[0m\n  \u001b[1;94m-->\u001b[0m  \u001b[4mschema.prisma:5\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\u001b[1;94m 4 | \u001b[0m                url = env(\"DBURL\")\n\u001b[1;94m 5 | \u001b[0m                directUrl = \u001b[1;91menv(\"DOES_NOT_EXIST\")\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\nValidation Error Count: 1","error_code":"P1012"}"#
-        ]];
+        let expected = expect![[r#"{"message":"\u001b[1;91merror\u001b[0m: \u001b[1mEnvironment variable not found: DOES_NOT_EXIST.\u001b[0m\n  \u001b[1;94m-->\u001b[0m  \u001b[4mschema.prisma:5\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\u001b[1;94m 4 | \u001b[0m                url = env(\"DBURL\")\n\u001b[1;94m 5 | \u001b[0m                \u001b[1;91mdirectUrl = env(\"DOES_NOT_EXIST\")\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\nValidation Error Count: 1","error_code":"P1012"}"#]];


The diff in this example is that before only the env(...) part was one format, and now the whole line it?

I'm honestly not sure how to read the diff here 😅

I think this is the additional line with a line number and the | that was removed - I commented on that below as well in a snapshot where it is more clearly visible.

(I think in prisam/prisma we use a stripAnsi function to remove the color codes from the messages and snapshot that as well - as it is more readable without breaking your brain)

prisma-fmt/src/text_document_completion/datasource.rs

janpio · 2023-03-03T16:43:41Z

prisma-fmt/src/text_document_completion/datasource.rs

+            kind: MarkupKind::Markdown,
+            value: generate_pretty_doc(
+                r#""connectionString""#,
+                r#"Connection URL including authentication info. Each datasource provider documents the URL syntax. Most providers use the syntax provided by the database. [Learn more](https://pris.ly/d/prisma-schema)."#,


Duplication with line 92 avoidable?

As an update to url

Connection URL to connect to your database. [Learn more](https://pris.ly/d/connection-strings). // instead of Connection URL including authentication info. Each datasource provider documents the URL syntax. Most providers use the syntax provided by the database. [Learn more](https://pris.ly/d/connection-strings).

prisma-fmt/tests/code_actions/scenarios/relation_mode_referential_integrity/result.json

janpio · 2023-03-03T16:45:36Z

prisma-fmt/src/validate.rs

-        let expected = expect![[
-            r#"{"error_code":"P1012","message":"\u001b[1;91merror\u001b[0m: \u001b[1mThe `referentialIntegrity` and `relationMode` attributes cannot be used together. Please use only `relationMode` instead.\u001b[0m\n  \u001b[1;94m-->\u001b[0m  \u001b[4mschema.prisma:6\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\u001b[1;94m 5 | \u001b[0m              relationMode = \"prisma\"\n\u001b[1;94m 6 | \u001b[0m              \u001b[1;91mreferentialIntegrity = \"foreignKeys\"\u001b[0m\n\u001b[1;94m 7 | \u001b[0m          }\n\u001b[1;94m   | \u001b[0m\n\nValidation Error Count: 1"}"#
-        ]];
+        let expected = expect![[r#"{"error_code":"P1012","message":"\u001b[1;91merror\u001b[0m: \u001b[1mThe `referentialIntegrity` and `relationMode` attributes cannot be used together. Please use only `relationMode` instead.\u001b[0m\n  \u001b[1;94m-->\u001b[0m  \u001b[4mschema.prisma:6\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\u001b[1;94m 5 | \u001b[0m              relationMode = \"prisma\"\n\u001b[1;94m 6 | \u001b[0m              \u001b[1;91mreferentialIntegrity = \"foreignKeys\"\u001b[0m\n\u001b[1;94m   | \u001b[0m\n\nValidation Error Count: 1"}"#]];


What is the differnece here? Can not figure it out visually.

this is the diff running cargo t locally

janpio · 2023-03-03T16:52:02Z

psl/psl/tests/config/datasources.rs

@@ -132,7 +132,6 @@ fn datasource_should_not_allow_arbitrary_parameters() {
        [1;94m   | [0m
        [1;94m 3 | [0m  url = "mysql://localhost"
        [1;94m 4 | [0m  [1;91mfoo = "bar"[0m
-        [1;94m 5 | [0m}


This additional line of context could in theory contain text and snippets from the schema, and be helpful this way. Did we intentionally remove this?

pimeys

Much better already. The specialization of certain completions is something we should have before we merge this. I'm willing to pair about this if needed. Just ping me.

pimeys · 2023-03-14T16:37:33Z

prisma-fmt/src/text_document_completion.rs

        position => ctx.connector().push_completions(ctx.db, position, completion_list),
    }
 }

+fn ds_has_prop(ctx: CompletionContext<'_>, prop: &str) -> bool {


I'd implement these directly in the datasource. So, we could ask from datasource ds.schemas_defined() and so on.

I'm not sure do we really need this. Maybe in a follow-up PR.

prisma-fmt/src/text_document_completion.rs

psl/schema-ast/src/ast/model.rs

This follows the changes for config blocks Co-authored-by: Tom Houlé <13155277+tomhoule@users.noreply.github.com>

Update test expects Co-authored-by: Tom Houlé <13155277+tomhoule@users.noreply.github.com>

new datasource positions

Added: new diagnostic for props without value Co-authored-by: Tom Houlé <13155277+tomhoule@users.noreply.github.com>

Added tests

env -> env() per what was in LT XYZ_URL kind -> 21, per what was in LT db gen > db pull completions no longer re-show-up formatting

- this will need to be re-added later when models and views are updated

Moved extensions and schemas (only postgres). Co-authored-by: Julius de Bruijn <bruijn@prisma.io>

- mssql, mysql, cockroachdb Moved connector completions to builtin-connectors crate

pimeys

a few things!

psl/builtin-connectors/src/cockroach_datamodel_connector.rs

prisma-fmt/src/text_document_completion.rs

pimeys · 2023-03-15T16:55:43Z

prisma-fmt/src/text_document_completion.rs

        position => ctx.connector().push_completions(ctx.db, position, completion_list),
    }
 }

+fn ds_has_prop(ctx: CompletionContext<'_>, prop: &str) -> bool {


I'm not sure do we really need this. Maybe in a follow-up PR.

psl/builtin-connectors/src/postgres_datamodel_connector.rs

psl/psl-core/src/configuration/datasource.rs

formatting

pimeys

Well done. To production we go!

This comment was marked as resolved.

Sign in to view

Druue added this to the 4.12.0 milestone Mar 2, 2023

Druue marked this pull request as ready for review March 3, 2023 12:04

Druue requested a review from a team as a code owner March 3, 2023 12:04

pimeys reviewed Mar 3, 2023

View reviewed changes

janpio reviewed Mar 3, 2023

View reviewed changes

prisma-fmt/src/text_document_completion/datasource.rs Outdated Show resolved Hide resolved

janpio reviewed Mar 3, 2023

View reviewed changes

prisma-fmt/src/text_document_completion/datasource.rs Outdated Show resolved Hide resolved

janpio reviewed Mar 3, 2023

View reviewed changes

prisma-fmt/tests/code_actions/scenarios/relation_mode_referential_integrity/result.json Show resolved Hide resolved

janpio reviewed Mar 3, 2023

View reviewed changes

Druue force-pushed the tech/spans branch 2 times, most recently from c21eb86 to 108cad8 Compare March 14, 2023 12:04

pimeys reviewed Mar 14, 2023

View reviewed changes

Druue and others added 13 commits March 15, 2023 17:21

Added datasource completions from LT to prisma-fmt

01a2ea3

Completions for DS now available at start of line

1856db1

ds completions no longer available outside of the closing curly-brace

1012933

Updated the rest of the grammar and parsing logic

6ef9feb

This follows the changes for config blocks Co-authored-by: Tom Houlé <13155277+tomhoule@users.noreply.github.com>

Updated reformatter to handle inner_contents

15134e0

Update test expects Co-authored-by: Tom Houlé <13155277+tomhoule@users.noreply.github.com>

fix prisma-fmt tests to reflect span changes

b9650cf

added tests for default ds completions and multischema

6af1430

Added: sub-completions for xyz_url

9e0d9e0

new datasource positions

Update: config values now Optional

167c549

Added: new diagnostic for props without value Co-authored-by: Tom Houlé <13155277+tomhoule@users.noreply.github.com>

Added tests for url argument completions

d84c5b9

Updated get_config test snapshots

3a3cc52

Added support for params to pretty_doc

b991179

Added XYZ_URL completion for env()

bd30177

Added tests

Druue and others added 17 commits March 15, 2023 17:21

Updated completion test snapshots due to params

92a535b

Remove engines tag from completion labels

20350aa

removed commented line

46e2010

update env doc to mention db pull

283bff0

extensions completion

3dc5864

validation to only offer completions for what's not already there

6b53257

add_quotes for envar name completions

4c9ec76

Updated Tests:

f3dd1d6

env -> env() per what was in LT XYZ_URL kind -> 21, per what was in LT db gen > db pull completions no longer re-show-up formatting

missed change in merge

4a31989

no fancy features allowed

02971bb

revert direct_url span back to just the arg

e2a088e

Clippy complaining about unused inner_span for models and views

c8890a7

- this will need to be re-added later when models and views are updated

formatting

687f0e1

Update ds and connector to check prop definitions

194ab42

cleanup unused

3762e2e

Added connector specific completion capabilities.

7c9018d

Moved extensions and schemas (only postgres). Co-authored-by: Julius de Bruijn <bruijn@prisma.io>

Add schemas completions to other connectors

4142420

- mssql, mysql, cockroachdb Moved connector completions to builtin-connectors crate

pimeys reviewed Mar 15, 2023

View reviewed changes

rename push_completions -> datamodel_completions

b8d6880

formatting

Druue force-pushed the tech/spans branch from bf1bdc0 to b8d6880 Compare March 15, 2023 18:48

pimeys approved these changes Mar 15, 2023

View reviewed changes

Druue merged commit 3b9f029 into main Mar 16, 2023

Druue deleted the tech/spans branch March 16, 2023 09:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Datasource Spans & Completions #3703

Datasource Spans & Completions #3703

Druue commented Feb 10, 2023 •

edited

Druue commented Feb 22, 2023

This comment was marked as resolved.

Druue commented Mar 3, 2023

pimeys left a comment

pimeys Mar 3, 2023

pimeys Mar 3, 2023

janpio Mar 3, 2023 •

edited

Druue Mar 3, 2023

janpio Mar 3, 2023

janpio Mar 3, 2023

janpio Mar 3, 2023

Druue Mar 13, 2023

janpio Mar 3, 2023

Druue Mar 13, 2023

janpio Mar 3, 2023

pimeys left a comment

pimeys Mar 14, 2023

pimeys Mar 15, 2023

pimeys left a comment

pimeys Mar 15, 2023

pimeys left a comment

Datasource Spans & Completions #3703

Datasource Spans & Completions #3703

Conversation

Druue commented Feb 10, 2023 • edited

Changes

Bugs

Todo

Druue commented Feb 22, 2023

This comment was marked as resolved.

Druue commented Mar 3, 2023

pimeys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janpio Mar 3, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pimeys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pimeys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pimeys left a comment

Choose a reason for hiding this comment

Druue commented Feb 10, 2023 •

edited

janpio Mar 3, 2023 •

edited