Skip to content

Commit

Permalink
Fix block attribute doc comment reformatting
Browse files Browse the repository at this point in the history
This is achieved by a massive cleanup and simplification of the
AST reformatter.

- The comments attached to block attributes will follow them after
  reformatting.
- Enum attributes do not get reformatted with an extra leading space
  anymore
- Empty lines at beginning and end of blocks are reformatted away.

closes prisma/language-tools#1186

Also included:

- diagnostics: delete dead warnings code and stop using thiserror —
  compile times will thank us.
  • Loading branch information
tomhoule committed Jul 4, 2022
1 parent 09f9659 commit b1d94f6
Show file tree
Hide file tree
Showing 38 changed files with 784 additions and 959 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ fn models_without_columns(datamodel: &mut Datamodel, ctx: &IntrospectionContext)
model.is_commented_out = true;
let comment = match ctx.sql_family().is_postgres() {
true =>
"We could not retrieve columns for the underlying table. Either it has none or you are missing rights to see them. Please check your privileges.".to_string(),
false=> "We could not retrieve columns for the underlying table. You probably have no rights to see them. Please check your privileges.".to_string(),
" We could not retrieve columns for the underlying table. Either it has none or you are missing rights to see them. Please check your privileges.".to_string(),
false=> " We could not retrieve columns for the underlying table. You probably have no rights to see them. Please check your privileges.".to_string(),

};
//postgres could be valid, or privileges, commenting out because we cannot handle it.
Expand All @@ -77,7 +77,7 @@ fn models_wihtout_uniques(datamodel: &mut Datamodel, models_without_columns: &[M
if model.strict_unique_criterias_disregarding_unsupported().is_empty() {
model.is_ignored = true;
model.documentation = Some(
"The underlying table does not contain a valid unique identifier and can therefore currently not be handled by the Prisma Client."
" The underlying table does not contain a valid unique identifier and can therefore currently not be handled by the Prisma Client."
.to_string(),
);
models_without_identifiers.push(Model {
Expand Down Expand Up @@ -110,7 +110,7 @@ fn fields_with_empty_names(datamodel: &mut Datamodel) -> Vec<ModelAndField> {
for field in model.scalar_fields_mut() {
if field.name.is_empty() {
field.documentation = Some(
"This field was commented out because of an invalid name. Please provide a valid one that matches [a-zA-Z][a-zA-Z0-9_]*"
" This field was commented out because of an invalid name. Please provide a valid one that matches [a-zA-Z][a-zA-Z0-9_]*"
.to_string(),
);
field.name = field.database_name.as_ref().unwrap().to_string();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ fn rename_reserved(model: &mut Model) {

if &name != model.name() {
let comment = format!(
"This model has been renamed to '{}' during introspection, because the original name '{}' is reserved.",
" This model has been renamed to '{}' during introspection, because the original name '{}' is reserved.",
name, model.name,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ async fn field_with_empty_name(api: &TestApi) -> TestResult {
let expectation = expect![[r#"
model A {
// This field was commented out because of an invalid name. Please provide a valid one that matches [a-zA-Z][a-zA-Z0-9_]*
// Int @default(autoincrement()) @map(" ")
// Int @default(autoincrement()) @map(" ")
}
"#]];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,45 @@ use test_macros::test_connector;

#[test_connector(tags(Postgres))]
async fn should_not_remap_if_renaming_would_lead_to_duplicate_names(api: &TestApi) -> TestResult {
api.database()
.raw_cmd("CREATE TABLE nodes(id serial primary key)")
.await?;
let sql = r#"
CREATE TABLE nodes(id serial primary key);
CREATE TABLE _nodes(
node_a int NOT NULL,
node_b int NOT NULL,
CONSTRAINT _nodes_node_a_fkey FOREIGN KEY(node_a) REFERENCES nodes(id) ON DELETE CASCADE ON UPDATE CASCADE,
CONSTRAINT _nodes_node_b_fkey FOREIGN KEY(node_b) REFERENCES nodes(id) ON DELETE CASCADE ON UPDATE CASCADE
);
"#;
api.raw_cmd(sql).await;

api.database()
.raw_cmd(
"CREATE TABLE _nodes(
node_a int not null,
node_b int not null,
constraint _nodes_node_a_fkey foreign key(node_a) references nodes(id) on delete cascade on update cascade,
constraint _nodes_node_b_fkey foreign key(node_b) references nodes(id) on delete cascade on update cascade
)
",
)
.await?;
let expected = expect![[r#"
generator client {
provider = "prisma-client-js"
}
datasource db {
provider = "postgresql"
url = "env(TEST_DATABASE_URL)"
}
model nodes {
id Int @id @default(autoincrement())
_nodes__nodes_node_aTonodes _nodes[] @relation("_nodes_node_aTonodes") @ignore
_nodes__nodes_node_bTonodes _nodes[] @relation("_nodes_node_bTonodes") @ignore
}
assert!(api.introspect().await.is_ok());
/// The underlying table does not contain a valid unique identifier and can therefore currently not be handled by the Prisma Client.
model _nodes {
node_a Int
node_b Int
nodes__nodes_node_aTonodes nodes @relation("_nodes_node_aTonodes", fields: [node_a], references: [id], onDelete: Cascade)
nodes__nodes_node_bTonodes nodes @relation("_nodes_node_bTonodes", fields: [node_b], references: [id], onDelete: Cascade)
@@ignore
}
"#]];
api.expect_datamodel(&expected).await;
Ok(())
}

Expand Down
15 changes: 7 additions & 8 deletions libs/datamodel/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,7 @@ pub fn parse_schema_parserdb(file: impl Into<SourceFile>) -> Result<ValidatedSch
let preview_features = preview_features(&generators);
let datasources = load_sources(db.ast(), preview_features, &mut diagnostics);

let out = validate(db, &datasources, preview_features, diagnostics);

let mut out = validate(db, &datasources, preview_features, diagnostics);
out.diagnostics
.to_result()
.map_err(|err| err.to_pretty_string("schema.prisma", file.as_str()))?;
Expand Down Expand Up @@ -178,7 +177,7 @@ fn parse_datamodel_internal(
},
datamodel,
),
warnings: out.diagnostics.warnings().to_vec(),
warnings: out.diagnostics.into_warnings(),
})
}

Expand Down Expand Up @@ -209,7 +208,7 @@ pub fn parse_configuration(schema: &str) -> Result<ValidatedConfiguration, diagn
generators,
datasources,
},
warnings: diagnostics.warnings().to_owned(),
warnings: diagnostics.into_warnings(),
})
}

Expand Down Expand Up @@ -248,11 +247,11 @@ pub fn render_datamodel_and_config_to_string(
}

/// Renders as a string into the stream.
fn render_schema_ast(schema: &ast::SchemaAst, ident_width: usize) -> String {
let mut out = String::with_capacity(schema.tops.len() * 20);
let mut renderer = schema_ast::renderer::Renderer::new(&mut out, ident_width);
fn render_schema_ast(schema: &ast::SchemaAst, indent_width: usize) -> String {
let mut renderer = schema_ast::renderer::Renderer::new(indent_width);
renderer.stream.reserve(schema.tops.len() * 20);
renderer.render(schema);
reformat(&out, 2).expect("Internal error: failed to reformat introspected schema")
reformat(&renderer.stream, 2).expect("Internal error: failed to reformat introspected schema")
}

fn preview_features(generators: &[Generator]) -> BitFlags<PreviewFeature> {
Expand Down
2 changes: 1 addition & 1 deletion libs/datamodel/core/src/mcf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ fn model_to_serializable(config: &ValidatedConfiguration) -> SerializeableMcf {
SerializeableMcf {
generators: generator::generators_to_json_value(&config.subject.generators),
datasources: source::render_sources_to_json_value(&config.subject.datasources),
warnings: config.warnings.iter().map(|f| f.description()).collect(),
warnings: config.warnings.iter().map(|f| f.message().to_owned()).collect(),
}
}
11 changes: 4 additions & 7 deletions libs/datamodel/core/src/transform/ast_to_dml/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,16 @@ pub fn parse_and_validate_preview_features(
preview_features: Vec<String>,
feature_map: &FeatureMap,
span: ast::Span,
) -> (Vec<PreviewFeature>, Diagnostics) {
let mut diagnostics = Diagnostics::new();
diagnostics: &mut Diagnostics,
) -> Vec<PreviewFeature> {
let mut features = vec![];

for feature_str in preview_features {
let feature_opt = PreviewFeature::parse_opt(&feature_str);
match feature_opt {
Some(feature) if feature_map.is_deprecated(&feature) => {
features.push(feature);
diagnostics.push_warning(DatamodelWarning::new_deprecated_preview_feature_warning(
&feature_str,
span,
))
diagnostics.push_warning(DatamodelWarning::new_feature_deprecated(&feature_str, span));
}

Some(feature) if !feature_map.is_valid(&feature) => {
Expand All @@ -42,5 +39,5 @@ pub fn parse_and_validate_preview_features(
}
}

(features, diagnostics)
features
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ impl GeneratorLoader {

let preview_features = match preview_features_arg {
Some((Ok(arr), span)) => {
let (features, mut diag) = parse_and_validate_preview_features(arr, &GENERATOR, span);
diagnostics.append(&mut diag);
let features = parse_and_validate_preview_features(arr, &GENERATOR, span, diagnostics);

Some(features)
}
Expand Down
4 changes: 2 additions & 2 deletions libs/datamodel/core/tests/base/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ fn parse_comments() {

let schema = parse(dml);
let user_model = schema.assert_has_model("User");
user_model.assert_with_documentation("The user model.");
user_model.assert_with_documentation(" The user model.");
user_model
.assert_has_scalar_field("firstName")
.assert_with_documentation("The first name.\nCan be multi-line.");
.assert_with_documentation(" The first name.\n Can be multi-line.");
}

#[test]
Expand Down
11 changes: 5 additions & 6 deletions libs/datamodel/core/tests/base/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ fn comments_must_work_in_models() {
"#;

let schema = parse(dml);
let user_model = schema.assert_has_model("User").assert_with_documentation("comment 1");
let user_model = schema.assert_has_model("User").assert_with_documentation(" comment 1");
user_model
.assert_has_scalar_field("firstName")
.assert_with_documentation("comment 3");
.assert_with_documentation(" comment 3");
user_model
.assert_has_scalar_field("lastName")
.assert_with_documentation("comment 4\ncomment 5");
.assert_with_documentation(" comment 4\n comment 5");
}

#[test]
Expand Down Expand Up @@ -87,13 +87,12 @@ fn comments_must_work_in_enums() {
// Comment below
}"#;

// must not crash
let schema = parse(dml);
schema
.assert_has_enum("Role")
.assert_with_documentation("Documentation Comment Enum")
.assert_with_documentation(" Documentation Comment Enum")
.assert_has_value("USER")
.assert_with_documentation("Documentation Comment Enum Value 1");
.assert_with_documentation(" Documentation Comment Enum Value 1");
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions libs/datamodel/core/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ impl ModelAsserts for dml::Model {
self
}

#[track_caller]
fn assert_with_documentation(&self, t: &str) -> &Self {
assert_eq!(self.documentation, Some(t.to_owned()));
self
Expand Down
12 changes: 5 additions & 7 deletions libs/datamodel/core/tests/config/nice_warnings.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::common::*;
use datamodel::ast::Span;
use datamodel::diagnostics::DatamodelWarning;
use diagnostics::{DatamodelWarning, Span};

#[test]
fn nice_warning_for_deprecated_generator_preview_feature() {
Expand All @@ -13,9 +12,8 @@ fn nice_warning_for_deprecated_generator_preview_feature() {

let res = datamodel::parse_configuration(schema).unwrap();

res.warnings
.assert_is(DatamodelWarning::new_deprecated_preview_feature_warning(
"middlewares",
Span::new(88, 103),
));
res.warnings.assert_is(DatamodelWarning::new_feature_deprecated(
"middlewares",
Span::new(88, 103),
));
}

0 comments on commit b1d94f6

Please sign in to comment.