Skip to content
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

support deprecated attribute in type constructors #3715

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,24 @@

### Compiler

- The compiler now allows deprecating specific custome type variants using the `@deprecated` attribute:

```gleam
pub type HashAlgor {
@deprecated("unsafe encription")
MD5
SHA224
SHA512
}

pub fn hash_password(input: String) -> String {
let results = hashing_lib.hash_input(input:, algo: MD5) // warning: MD5 is depreacated
results.hash
}
```

([Iesha](https://github.com/wilbert-mad))

- The compiler now prints correctly qualified or aliased type names when
printing type errors.

Expand Down
52 changes: 49 additions & 3 deletions compiler-core/src/analyse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
} => self.track_feature_usage(FeatureKind::InternalAnnotation, location),
}

let constructors = constructors
let constructors: Vec<RecordConstructor<Arc<Type>>> = constructors
.into_iter()
.map(
|RecordConstructor {
Expand All @@ -832,8 +832,15 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
name,
arguments: args,
documentation,
deprecation: constructor_deprecation,
}| {
self.check_name_case(name_location, &name, Named::CustomTypeVariant);
if constructor_deprecation.is_deprecated() {
self.track_feature_usage(
FeatureKind::VariantWithDeprecatedAnnotation,
location,
);
}

let preregistered_fn = environment
.get_variable(&name)
Expand Down Expand Up @@ -868,6 +875,7 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
name,
arguments: args,
documentation,
deprecation: constructor_deprecation,
}
},
)
Expand All @@ -878,6 +886,36 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
.parameters
.clone();

// Check if all constructors are deprecated if so error.
if !constructors.is_empty()
&& constructors
.iter()
.all(|record| record.deprecation.is_deprecated())
{
self.problems
.error(Error::AllVariantsDeprecated { location });
}

// If any constructor record/varient is deprecated while
// the type is deprecated as a whole that is considered an error.
if deprecation.is_deprecated()
&& !constructors.is_empty()
&& constructors
.iter()
.any(|record| record.deprecation.is_deprecated())
{
// Report error on all variants attibuted with deprecated
constructors
.iter()
.filter(|record| record.deprecation.is_deprecated())
.for_each(|record| {
self.problems
.error(Error::DeprecatedVariantOnDeprecatedType {
location: record.location,
});
});
}

Ok(Definition::CustomType(CustomType {
documentation: doc,
location,
Expand Down Expand Up @@ -1014,11 +1052,19 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
*publicity
};

// If the whole custom type is deprecated all of its varints are too.
// Otherwise just the varint(s) attributed as deprecated are.
let deprecate_constructor = if deprecation.is_deprecated() {
deprecation
} else {
&constructor.deprecation
};

environment.insert_module_value(
constructor.name.clone(),
ValueConstructor {
publicity: value_constructor_publicity,
deprecation: deprecation.clone(),
deprecation: deprecate_constructor.clone(),
type_: type_.clone(),
variant: constructor_info.clone(),
},
Expand All @@ -1042,7 +1088,7 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
constructor_info,
type_,
value_constructor_publicity,
deprecation.clone(),
deprecate_constructor.clone(),
);

environment.names.named_constructor_in_scope(
Expand Down
1 change: 1 addition & 0 deletions compiler-core/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ pub struct RecordConstructor<T> {
pub name: EcoString,
pub arguments: Vec<RecordConstructorArg<T>>,
pub documentation: Option<(u32, EcoString)>,
pub deprecation: Deprecation,
}

impl<A> RecordConstructor<A> {
Expand Down
46 changes: 46 additions & 0 deletions compiler-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3129,6 +3129,52 @@ Try: _{}", kind_str.to_title_case(), name.to_snake_case()),
}),
}
},
TypeError::AllVariantsDeprecated { location } => {
let text = String::from("Consider deprecating the type as a whole.

@deprecated(\"message\")
type Wibble {
Wobble1
Wobble2
}
");
Diagnostic {
title: "All variants of custom type deprecated.".into(),
text,
hint: None,
level: Level::Error,
location: Some(Location {
label: Label {
text: None,
span: *location,
},
path: path.clone(),
src: src.clone(),
extra_labels: vec![],
})
}
},
TypeError::DeprecatedVariantOnDeprecatedType{ location } => {
let text = wrap("This custom type has already been deprecated, so deprecating \
one of its variants does nothing.
Consider removing the deprecation attribute on the variant.");

Diagnostic {
title: "Custom type already deprecated".into(),
text,
hint: None,
level: Level::Error,
location: Some(Location {
label: Label {
text: None,
span: *location,
},
path: path.clone(),
src: src.clone(),
extra_labels: vec![],
})
}
}
}
}).collect_vec(),

Expand Down
21 changes: 10 additions & 11 deletions compiler-core/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1566,17 +1566,18 @@ impl<'comments> Formatter<'comments> {
) -> Document<'a> {
let comments = self.pop_comments(constructor.location.start);
let doc_comments = self.doc_comments(constructor.location.start);
let attributes = AttributesPrinter::new()
.set_deprecation(&constructor.deprecation)
.to_doc();

let doc = if constructor.arguments.is_empty() {
if self.any_comments(constructor.location.end) {
constructor
.name
.as_str()
.to_doc()
attributes
.append(constructor.name.as_str().to_doc())
.append(self.wrap_args(vec![], constructor.location.end))
.group()
} else {
constructor.name.as_str().to_doc()
attributes.append(constructor.name.as_str().to_doc())
}
} else {
let args = constructor
Expand All @@ -1602,12 +1603,10 @@ impl<'comments> Formatter<'comments> {
},
)
.collect_vec();
constructor
.name
.as_str()
.to_doc()
.append(self.wrap_args(args, constructor.location.end))
.group()

Wilbert-mad marked this conversation as resolved.
Show resolved Hide resolved
attributes
.append(constructor.name.as_str().to_doc())
.append(self.wrap_args(args, constructor.location.end).group())
};

commented(doc_comments.append(doc).group(), comments)
Expand Down
2 changes: 1 addition & 1 deletion compiler-core/src/language_server/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ fn custom_type_symbol(
} else {
SymbolKind::CONSTRUCTOR
},
tags: None,
tags: make_deprecated_symbol_tag(&constructor.deprecation),
deprecated: None,
range: src_span_to_lsp_range(full_constructor_span, line_numbers),
selection_range: src_span_to_lsp_range(constructor.name_location, line_numbers),
Expand Down
20 changes: 20 additions & 0 deletions compiler-core/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2145,6 +2145,24 @@ where
let constructors = Parser::series_of(
self,
&|p| {
// The only attribute supported on constructors is @deprecated
let mut attributes = Attributes::default();
let attr_loc = Parser::parse_attributes(p, &mut attributes)?;

if let Some(attr_span) = attr_loc {
// Expecting all but the deprecated atterbutes to be default
if attributes.external_erlang.is_some()
|| attributes.external_javascript.is_some()
|| attributes.target.is_some()
|| attributes.internal != InternalAttribute::Missing
{
return parse_error(
ParseErrorType::UnknownAttributeRecordVariant,
attr_span,
);
}
}

if let Some((c_s, c_n, c_e)) = Parser::maybe_upname(p) {
let documentation = p.take_documentation(c_s);
let (args, args_e) = Parser::parse_type_constructor_args(p)?;
Expand All @@ -2158,6 +2176,7 @@ where
name: c_n,
arguments: args,
documentation,
deprecation: attributes.deprecated,
}))
} else {
Ok(None)
Expand Down Expand Up @@ -2193,6 +2212,7 @@ where
} else {
(vec![], end)
};

Ok(Some(Definition::CustomType(CustomType {
documentation,
location: SrcSpan { start, end },
Expand Down
5 changes: 5 additions & 0 deletions compiler-core/src/parse/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,10 @@ utf16_codepoint, utf32_codepoint, signed, unsigned, big, little, native, size, u
"Hint: If a type is not generic you should omit the `()`.".into(),
],
),
ParseErrorType::UnknownAttributeRecordVariant => (
"This attribute cannot be used on a variant.",
vec!["Hint: Did you mean `@deprecated`?".into()],
),
}
}
}
Expand Down Expand Up @@ -398,6 +402,7 @@ pub enum ParseErrorType {
ConstantRecordConstructorNoArguments, // const x = Record()
TypeConstructorNoArguments, // let a : Int()
TypeDefinitionNoArguments, // pub type Wibble() { ... }
UnknownAttributeRecordVariant, // an attribute was used that is not know for a custom type variant
}

impl LexicalError {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
---
source: compiler-core/src/parse/tests.rs
assertion_line: 1512
expression: "\ntype Wibble {\n @deprecated(\"1\")\n Wibble1\n Wibble2\n}\n"
---
Parsed {
module: Module {
name: "",
documentation: [],
type_info: (),
definitions: [
TargetedDefinition {
definition: CustomType(
CustomType {
location: SrcSpan {
start: 1,
end: 12,
},
end_position: 61,
name: "Wibble",
name_location: SrcSpan {
start: 6,
end: 12,
},
publicity: Private,
constructors: [
RecordConstructor {
location: SrcSpan {
start: 40,
end: 47,
},
name_location: SrcSpan {
start: 40,
end: 47,
},
name: "Wibble1",
arguments: [],
documentation: None,
deprecation: Deprecated {
message: "1",
},
},
RecordConstructor {
location: SrcSpan {
start: 52,
end: 59,
},
name_location: SrcSpan {
start: 52,
end: 59,
},
name: "Wibble2",
arguments: [],
documentation: None,
deprecation: NotDeprecated,
},
],
documentation: None,
deprecation: NotDeprecated,
opaque: false,
parameters: [],
typed_parameters: [],
},
),
target: None,
},
],
names: Names {
local_types: {},
imported_modules: {},
type_variables: {},
local_value_constructors: {},
},
},
extra: ModuleExtra {
module_comments: [],
doc_comments: [],
comments: [],
empty_lines: [],
new_lines: [
0,
14,
35,
47,
59,
61,
],
},
}
Loading