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 8326415 commit 8cbc3fd
Show file tree
Hide file tree
Showing 37 changed files with 748 additions and 944 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
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),
));
}
87 changes: 68 additions & 19 deletions libs/datamodel/core/tests/reformat/reformat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,8 @@ fn end_of_line_comments_must_not_influence_table_layout_in_enums() {

let expected = expect![[r#"
enum Foo {
ONE @map("short") // COMMENT 1
TWO @map("a_very_long_name") // COMMENT 2
ONE @map("short") // COMMENT 1
TWO @map("a_very_long_name") // COMMENT 2
}
"#]];

Expand Down Expand Up @@ -420,8 +420,8 @@ fn test_floating_doc_comments_1() {
model a {
one Int
two Int
// bs b[] @relation(references: [a])
// bs b[] @relation(references: [a])
@@id([one, two])
}
Expand Down Expand Up @@ -480,12 +480,12 @@ fn reformatting_enums_must_work() {

let expected = expect![[r#"
enum Colors {
RED @map("rett")
RED @map("rett")
BLUE
GREEN
// comment
ORANGE_AND_KIND_OF_RED @map("super_color")
ORANGE_AND_KIND_OF_RED @map("super_color")
@@map("the_colors")
}
Expand Down Expand Up @@ -629,7 +629,7 @@ fn incomplete_field_definitions_in_a_model_must_not_get_removed() {
}

#[test]
fn new_lines_inside_block_above_field_must_stay() {
fn new_lines_inside_block_above_field_must_go_away() {
let input = indoc! {r#"
model Post {
Expand All @@ -642,10 +642,6 @@ fn new_lines_inside_block_above_field_must_stay() {

let expected = expect![[r#"
model Post {
id Int @id @default(autoincrement())
}
"#]];
Expand All @@ -654,7 +650,7 @@ fn new_lines_inside_block_above_field_must_stay() {
}

#[test]
fn new_lines_inside_block_below_field_must_stay() {
fn new_lines_inside_block_below_field_must_go_away() {
let input = indoc! {r#"
model Post {
id Int @id @default(autoincrement())
Expand All @@ -668,18 +664,14 @@ fn new_lines_inside_block_below_field_must_stay() {
let expected = expect![[r#"
model Post {
id Int @id @default(autoincrement())
}
"#]];

expected.assert_eq(&reformat(input));
}

#[test]
fn new_lines_inside_block_in_between_fields_must_stay() {
fn new_lines_inside_block_in_between_fields_must_go_away() {
let input = indoc! {r#"
model Post {
id Int @id @default(autoincrement())
Expand All @@ -694,9 +686,7 @@ fn new_lines_inside_block_in_between_fields_must_stay() {
model Post {
id Int @id @default(autoincrement())
input String
}
"#]];

Expand Down Expand Up @@ -809,7 +799,6 @@ fn multiple_new_lines_between_top_level_elements_must_be_reduced_to_a_single_one
}
"#};

// TODO: the formatting of the type alias is not nice
let expected = expect![[r#"
model Post {
id Int @id
Expand Down Expand Up @@ -907,6 +896,7 @@ fn incomplete_last_line_must_not_stop_reformatting() {
model User {
id Int @id
}
model Bl
"#]];

Expand Down Expand Up @@ -1450,3 +1440,62 @@ fn attribute_arguments_reformatting_is_idempotent() {
expected.assert_eq(&reformatted);
assert_eq!(reformatted, reformat(&reformatted)); // it's idempotent
}

#[test]
fn block_attribute_comments_are_preserved() {
let schema = r#"
model MyModel {
// Example Comment
field1 Int // Comment Field 1
field2 Int // Comment Field 2
field3 Int // Comment Field 3
// This comment should stay here.
@@unique([field1, field2]) // younique
// This one must stay too.
@@unique([field1, field3]) // there too
}
type YourType {
// Example Comment
field1 Int // Comment Field 1
field2 Int // Comment Field 2
field3 Int // Comment Field 3
// This comment should stay here.
@@unique([field1, field2]) // younique
// This one must stay too.
@@unique([field1, field3]) // there too
}
"#;

let expected = expect![[r#"
model MyModel {
// Example Comment
field1 Int // Comment Field 1
field2 Int // Comment Field 2
field3 Int // Comment Field 3
// This comment should stay here.
@@unique([field1, field2]) // younique
// This one must stay too.
@@unique([field1, field3]) // there too
}
type YourType {
// Example Comment
field1 Int // Comment Field 1
field2 Int // Comment Field 2
field3 Int // Comment Field 3
// This comment should stay here.
@@unique([field1, field2]) // younique
// This one must stay too.
@@unique([field1, field3]) // there too
}
"#]];

expected.assert_eq(&reformat(schema));
}
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,7 @@ fn should_add_back_relations_for_more_complex_cases() {
post Post @relation(fields: [postId], references: [post_id])
category Category @relation(fields: [categoryId], references: [category_id])
@@map("post_to_category")
}
"#]];
Expand Down

0 comments on commit 8cbc3fd

Please sign in to comment.