From abf408d732f2f0ba5e01be91b494877ca6977295 Mon Sep 17 00:00:00 2001 From: Chris Chang <51393127+chriscerie@users.noreply.github.com> Date: Mon, 29 Apr 2024 03:41:52 +0900 Subject: [PATCH 1/4] Support specific params to be deprecated and properly deprecate instance.new's second param (#594) Closes #436, closes #433. --- CHANGELOG.md | 2 + docs/src/usage/std.md | 2 +- selene-lib/default_std/roblox_base.yml | 5 ++ selene-lib/src/lints/deprecated.rs | 69 ++++++++++++++++--- selene-lib/src/standard_library/mod.rs | 4 ++ selene-lib/src/standard_library/v1_upgrade.rs | 1 + .../lints/deprecated/deprecated_params.lua | 8 +++ .../deprecated/deprecated_params.std.yml | 17 +++++ .../lints/deprecated/deprecated_params.stderr | 40 +++++++++++ .../tests/lints/deprecated/specific_allow.lua | 2 + .../lints/deprecated/specific_allow.std.yml | 7 ++ selene/src/roblox/generate_std.rs | 24 +++++-- 12 files changed, 167 insertions(+), 14 deletions(-) create mode 100644 selene-lib/tests/lints/deprecated/deprecated_params.lua create mode 100644 selene-lib/tests/lints/deprecated/deprecated_params.std.yml create mode 100644 selene-lib/tests/lints/deprecated/deprecated_params.stderr 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, From 49cc8411f25afbbda5a1e973bf20fc2c25a1943c Mon Sep 17 00:00:00 2001 From: Kampfkarren Date: Sun, 28 Apr 2024 11:54:06 -0700 Subject: [PATCH 2/4] 0.27.0 [release] --- CHANGELOG.md | 2 ++ Cargo.lock | 4 ++-- Cargo.toml | 4 ++-- selene/Cargo.toml | 4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49302b86..5074d837 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased](https://github.com/Kampfkarren/selene/compare/0.26.1...HEAD) + +## [0.27.0](https://github.com/Kampfkarren/selene/compare/0.26.1...0.26.2) - 2024-04-28 ### Added - Added `CFrame.lookAlong` to the Roblox standard library - Added `deprecated` config field to standard library function parameters diff --git a/Cargo.lock b/Cargo.lock index d312b6ff..84ecc886 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -941,7 +941,7 @@ dependencies = [ [[package]] name = "selene" -version = "0.26.1" +version = "0.27.0" dependencies = [ "atty", "cfg-if", @@ -971,7 +971,7 @@ dependencies = [ [[package]] name = "selene-lib" -version = "0.26.1" +version = "0.27.0" dependencies = [ "codespan", "codespan-reporting", diff --git a/Cargo.toml b/Cargo.toml index 13c090a1..049fec09 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = ["selene", "selene-lib"] resolver = "2" [workspace.package] -version = "0.26.1" +version = "0.27.0" authors = ["Kampfkarren "] edition = "2021" homepage = "https://kampfkarren.github.io/selene/" @@ -15,4 +15,4 @@ full_moon = "0.19.0" toml = "0.7.2" # Do not update this without confirming profiling uses the same version of tracy-client as selene -profiling = "1.0.7" \ No newline at end of file +profiling = "1.0.7" diff --git a/selene/Cargo.toml b/selene/Cargo.toml index 7ad931a3..05885800 100644 --- a/selene/Cargo.toml +++ b/selene/Cargo.toml @@ -24,7 +24,7 @@ globset = "0.4.10" lazy_static = "1.4" num_cpus = "1.15" profiling.workspace = true -selene-lib = { path = "../selene-lib", version = "=0.26.1", default-features = false } +selene-lib = { path = "../selene-lib", version = "=0.27.0", default-features = false } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" serde_yaml = "0.9.16" @@ -42,4 +42,4 @@ pretty_assertions = "1.3" [features] default = ["roblox"] tracy-profiling = ["profiling/profile-with-tracy", "tracy-client"] -roblox = ["selene-lib/roblox", "full_moon/roblox", "ureq"] \ No newline at end of file +roblox = ["selene-lib/roblox", "full_moon/roblox", "ureq"] From cd08fc4e283197a47dbb6f2fc8b31134da656083 Mon Sep 17 00:00:00 2001 From: Kampfkarren Date: Sun, 28 Apr 2024 13:06:15 -0700 Subject: [PATCH 3/4] Fix Instance.new second parameter being required --- selene/src/roblox/generate_std.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selene/src/roblox/generate_std.rs b/selene/src/roblox/generate_std.rs index 95481024..5a7384c2 100644 --- a/selene/src/roblox/generate_std.rs +++ b/selene/src/roblox/generate_std.rs @@ -270,7 +270,7 @@ impl RobloxGenerator { }, Argument { argument_type: ArgumentType::Display("Instance".to_string()), - required: Required::Required(None), + required: Required::NotRequired, observes: Observes::ReadWrite, deprecated: Some(Deprecated { message: "set the instance's parent separately".to_owned(), From cbd7746b499ec88e88fa54cd841a6cb4ead2aa8a Mon Sep 17 00:00:00 2001 From: Kampfkarren Date: Sun, 28 Apr 2024 13:07:25 -0700 Subject: [PATCH 4/4] 0.27.1 [release] --- CHANGELOG.md | 8 ++++++-- Cargo.lock | 4 ++-- Cargo.toml | 2 +- selene/Cargo.toml | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5074d837..ad388a10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,13 @@ # Changelog This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased](https://github.com/Kampfkarren/selene/compare/0.26.1...HEAD) +## [Unreleased](https://github.com/Kampfkarren/selene/compare/0.27.1...HEAD) -## [0.27.0](https://github.com/Kampfkarren/selene/compare/0.26.1...0.26.2) - 2024-04-28 +## [0.27.1](https://github.com/Kampfkarren/selene/releases/tag/0.27.1) - 2024-04-28 +### Fixed +- Fixed `Instance.new`'s second parameter being incorrectly marked as required. + +## [0.27.0](https://github.com/Kampfkarren/selene/releases/tag/0.27.0) - 2024-04-28 ### Added - Added `CFrame.lookAlong` to the Roblox standard library - Added `deprecated` config field to standard library function parameters diff --git a/Cargo.lock b/Cargo.lock index 84ecc886..3b8a6f39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -941,7 +941,7 @@ dependencies = [ [[package]] name = "selene" -version = "0.27.0" +version = "0.27.1" dependencies = [ "atty", "cfg-if", @@ -971,7 +971,7 @@ dependencies = [ [[package]] name = "selene-lib" -version = "0.27.0" +version = "0.27.1" dependencies = [ "codespan", "codespan-reporting", diff --git a/Cargo.toml b/Cargo.toml index 049fec09..caf6734f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = ["selene", "selene-lib"] resolver = "2" [workspace.package] -version = "0.27.0" +version = "0.27.1" authors = ["Kampfkarren "] edition = "2021" homepage = "https://kampfkarren.github.io/selene/" diff --git a/selene/Cargo.toml b/selene/Cargo.toml index 05885800..e5c08751 100644 --- a/selene/Cargo.toml +++ b/selene/Cargo.toml @@ -24,7 +24,7 @@ globset = "0.4.10" lazy_static = "1.4" num_cpus = "1.15" profiling.workspace = true -selene-lib = { path = "../selene-lib", version = "=0.27.0", default-features = false } +selene-lib = { path = "../selene-lib", version = "=0.27.1", default-features = false } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" serde_yaml = "0.9.16"