diff --git a/CHANGELOG.md b/CHANGELOG.md index 6250d33a..49302b86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased](https://github.com/Kampfkarren/selene/compare/0.26.1...HEAD) ### Added - Added `CFrame.lookAlong` to the Roblox standard library +- Added `deprecated` config field to standard library function parameters ### Changed - Updated the warning message for the `mixed_table` lint to include why mixed tables should be avoided +- Properly deprecated `Instance.new`'s second argument in the Roblox standard library ## [0.26.1](https://github.com/Kampfkarren/selene/releases/tag/0.26.1) - 2023-11-11 ### Fixed diff --git a/docs/src/usage/std.md b/docs/src/usage/std.md index 93e06247..70f21a80 100644 --- a/docs/src/usage/std.md +++ b/docs/src/usage/std.md @@ -154,7 +154,7 @@ globals: A field is understood as a table if it has fields of its own. Notice that `math` is not defined anywhere, but its fields are. This will create an implicit `math` with the property writability of `read-only`. ### Deprecated -Any field can have a deprecation notice added to it, which will then be read by [the deprecated lint](../lints/deprecated.md). +Any field or arg can have a deprecation notice added to it, which will then be read by [the deprecated lint](../lints/deprecated.md). ```yaml --- diff --git a/selene-lib/default_std/roblox_base.yml b/selene-lib/default_std/roblox_base.yml index b8608e0d..6d99221b 100644 --- a/selene-lib/default_std/roblox_base.yml +++ b/selene-lib/default_std/roblox_base.yml @@ -320,6 +320,11 @@ globals: Instance.new: args: - type: string + - required: false + type: + display: Instance + deprecated: + message: set the instance's parent separately # This is only must_use because we don't allow the second parameter must_use: true NumberRange.new: diff --git a/selene-lib/src/lints/deprecated.rs b/selene-lib/src/lints/deprecated.rs index 46644444..262557d3 100644 --- a/selene-lib/src/lints/deprecated.rs +++ b/selene-lib/src/lints/deprecated.rs @@ -3,7 +3,7 @@ use std::convert::Infallible; use full_moon::{ast, visitors::Visitor}; use serde::Deserialize; -use crate::ast_util::{name_paths::*, scopes::ScopeManager}; +use crate::ast_util::{name_paths::*, range, scopes::ScopeManager}; use super::{super::standard_library::*, *}; @@ -48,6 +48,11 @@ struct DeprecatedVisitor<'a> { standard_library: &'a StandardLibrary, } +struct Argument { + display: String, + range: (usize, usize), +} + impl<'a> DeprecatedVisitor<'a> { fn new( config: &DeprecatedLintConfig, @@ -94,7 +99,7 @@ impl<'a> DeprecatedVisitor<'a> { node: &N, what: &str, name_path: &[String], - parameters: &[String], + arguments: &[Argument], ) { assert!(!name_path.is_empty()); @@ -115,7 +120,12 @@ impl<'a> DeprecatedVisitor<'a> { let mut notes = vec![deprecated.message.to_owned()]; - if let Some(replace_with) = deprecated.try_instead(parameters) { + if let Some(replace_with) = deprecated.try_instead( + &arguments + .iter() + .map(|arg| arg.display.clone()) + .collect::>(), + ) { notes.push(format!("try: {replace_with}")); } @@ -130,6 +140,28 @@ impl<'a> DeprecatedVisitor<'a> { Vec::new(), )); } + + if let Some(Field { + field_kind: FieldKind::Function(function), + .. + }) = self.standard_library.find_global(name_path) + { + for (arg, arg_std) in arguments + .iter() + .zip(&function.arguments) + .filter(|(arg, _)| arg.display != "nil") + { + if let Some(deprecated) = &arg_std.deprecated { + self.diagnostics.push(Diagnostic::new_complete( + "deprecated", + "this parameter is deprecated".to_string(), + Label::new(arg.range), + vec![deprecated.message.clone()], + Vec::new(), + )); + }; + } + } } } @@ -194,21 +226,32 @@ impl Visitor for DeprecatedVisitor<'_> { feature = "force_exhaustive_checks", deny(non_exhaustive_omitted_patterns) )] - let argument_displays = match function_args { + let arguments = match function_args { ast::FunctionArgs::Parentheses { arguments, .. } => arguments .iter() - .map(|argument| argument.to_string()) + .map(|argument| Argument { + display: argument.to_string().trim_end().to_string(), + range: range(argument), + }) .collect(), - ast::FunctionArgs::String(token) => vec![token.to_string()], + ast::FunctionArgs::String(token) => vec![ + (Argument { + display: token.to_string(), + range: range(token), + }), + ], ast::FunctionArgs::TableConstructor(table_constructor) => { - vec![table_constructor.to_string()] + vec![Argument { + display: table_constructor.to_string(), + range: range(table_constructor), + }] } _ => Vec::new(), }; - self.check_name_path(call, "function", &name_path, &argument_displays); + self.check_name_path(call, "function", &name_path, &arguments); } } @@ -234,6 +277,15 @@ mod tests { ); } + #[test] + fn test_deprecated_params() { + test_lint( + DeprecatedLint::new(DeprecatedLintConfig::default()).unwrap(), + "deprecated", + "deprecated_params", + ); + } + #[test] fn test_specific_allow() { test_lint( @@ -242,6 +294,7 @@ mod tests { "deprecated_allowed".to_owned(), "more.*".to_owned(), "wow.*.deprecated_allowed".to_owned(), + "deprecated_param".to_owned(), ], }) .unwrap(), diff --git a/selene-lib/src/standard_library/mod.rs b/selene-lib/src/standard_library/mod.rs index 77448e95..79ff08f5 100644 --- a/selene-lib/src/standard_library/mod.rs +++ b/selene-lib/src/standard_library/mod.rs @@ -603,6 +603,10 @@ pub struct Argument { #[serde(default)] #[serde(skip_serializing_if = "is_default")] pub observes: Observes, + + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub deprecated: Option, } #[derive(Clone, Debug, Hash, PartialEq, Eq)] diff --git a/selene-lib/src/standard_library/v1_upgrade.rs b/selene-lib/src/standard_library/v1_upgrade.rs index 176fc563..3bbf0d87 100644 --- a/selene-lib/src/standard_library/v1_upgrade.rs +++ b/selene-lib/src/standard_library/v1_upgrade.rs @@ -41,6 +41,7 @@ impl From for Argument { required: v1_argument.required.into(), argument_type: v1_argument.argument_type.into(), observes: Observes::ReadWrite, + deprecated: None, } } } diff --git a/selene-lib/tests/lints/deprecated/deprecated_params.lua b/selene-lib/tests/lints/deprecated/deprecated_params.lua new file mode 100644 index 00000000..5e58e59e --- /dev/null +++ b/selene-lib/tests/lints/deprecated/deprecated_params.lua @@ -0,0 +1,8 @@ +local _ = Instance.new(a) +local _ = Instance.new(a, b) +local _ = Instance.new(a, nil ) +local _ = Instance.new(a, "nil") + +a(1) +a "" +a {} diff --git a/selene-lib/tests/lints/deprecated/deprecated_params.std.yml b/selene-lib/tests/lints/deprecated/deprecated_params.std.yml new file mode 100644 index 00000000..3dc6f959 --- /dev/null +++ b/selene-lib/tests/lints/deprecated/deprecated_params.std.yml @@ -0,0 +1,17 @@ +--- +globals: + Instance.new: + args: + - type: string + - required: false + type: + display: Instance + deprecated: + message: set the instance's parent separately + a: + args: + - required: false + type: + display: any + deprecated: + message: this is deprecated diff --git a/selene-lib/tests/lints/deprecated/deprecated_params.stderr b/selene-lib/tests/lints/deprecated/deprecated_params.stderr new file mode 100644 index 00000000..6e29413d --- /dev/null +++ b/selene-lib/tests/lints/deprecated/deprecated_params.stderr @@ -0,0 +1,40 @@ +error[deprecated]: this parameter is deprecated + ┌─ deprecated_params.lua:2:27 + │ +2 │ local _ = Instance.new(a, b) + │ ^ + │ + = set the instance's parent separately + +error[deprecated]: this parameter is deprecated + ┌─ deprecated_params.lua:4:27 + │ +4 │ local _ = Instance.new(a, "nil") + │ ^^^^^ + │ + = set the instance's parent separately + +error[deprecated]: this parameter is deprecated + ┌─ deprecated_params.lua:6:3 + │ +6 │ a(1) + │ ^ + │ + = this is deprecated + +error[deprecated]: this parameter is deprecated + ┌─ deprecated_params.lua:7:3 + │ +7 │ a "" + │ ^^ + │ + = this is deprecated + +error[deprecated]: this parameter is deprecated + ┌─ deprecated_params.lua:8:3 + │ +8 │ a {} + │ ^^ + │ + = this is deprecated + diff --git a/selene-lib/tests/lints/deprecated/specific_allow.lua b/selene-lib/tests/lints/deprecated/specific_allow.lua index 71671714..40a07f75 100644 --- a/selene-lib/tests/lints/deprecated/specific_allow.lua +++ b/selene-lib/tests/lints/deprecated/specific_allow.lua @@ -5,3 +5,5 @@ more.deprecated_allowed() wow.extra.deprecated_allowed() deprecated_allowed.more() + +deprecated_param(1) diff --git a/selene-lib/tests/lints/deprecated/specific_allow.std.yml b/selene-lib/tests/lints/deprecated/specific_allow.std.yml index f31f8cb6..ba66141a 100644 --- a/selene-lib/tests/lints/deprecated/specific_allow.std.yml +++ b/selene-lib/tests/lints/deprecated/specific_allow.std.yml @@ -16,3 +16,10 @@ globals: args: [] deprecated: message: "this is deprecated" + deprecated_param: + args: + - required: false + type: + display: any + deprecated: + message: this is deprecated diff --git a/selene/src/roblox/generate_std.rs b/selene/src/roblox/generate_std.rs index 89c402af..95481024 100644 --- a/selene/src/roblox/generate_std.rs +++ b/selene/src/roblox/generate_std.rs @@ -125,6 +125,7 @@ impl RobloxGenerator { argument_type: ArgumentType::Any, required: Required::NotRequired, observes: Observes::ReadWrite, + deprecated: None, }) .collect(), method: true, @@ -260,11 +261,23 @@ impl RobloxGenerator { self.std.globals.insert( "Instance.new".to_owned(), Field::from_field_kind(FieldKind::Function(FunctionBehavior { - arguments: vec![Argument { - argument_type: ArgumentType::Constant(instance_names), - required: Required::Required(None), - observes: Observes::ReadWrite, - }], + arguments: vec![ + Argument { + argument_type: ArgumentType::Constant(instance_names), + required: Required::Required(None), + observes: Observes::ReadWrite, + deprecated: None, + }, + Argument { + argument_type: ArgumentType::Display("Instance".to_string()), + required: Required::Required(None), + observes: Observes::ReadWrite, + deprecated: Some(Deprecated { + message: "set the instance's parent separately".to_owned(), + replace: vec![], + }), + }, + ], method: false, // Only true because we don't allow the second parameter @@ -294,6 +307,7 @@ impl RobloxGenerator { argument_type: ArgumentType::Constant(service_names), required: Required::Required(None), observes: Observes::ReadWrite, + deprecated: None, }], method: true, must_use: true,