Skip to content

Commit

Permalink
Support specific params to be deprecated and properly deprecate insta…
Browse files Browse the repository at this point in the history
…nce.new's second param (#594)

Closes #436, closes #433.
chriscerie authored Apr 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent e73cccc commit abf408d
Showing 12 changed files with 167 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion docs/src/usage/std.md
Original file line number Diff line number Diff line change
@@ -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
---
5 changes: 5 additions & 0 deletions selene-lib/default_std/roblox_base.yml
Original file line number Diff line number Diff line change
@@ -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:
69 changes: 61 additions & 8 deletions selene-lib/src/lints/deprecated.rs
Original file line number Diff line number Diff line change
@@ -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::<Vec<_>>(),
) {
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(),
4 changes: 4 additions & 0 deletions selene-lib/src/standard_library/mod.rs
Original file line number Diff line number Diff line change
@@ -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<Deprecated>,
}

#[derive(Clone, Debug, Hash, PartialEq, Eq)]
1 change: 1 addition & 0 deletions selene-lib/src/standard_library/v1_upgrade.rs
Original file line number Diff line number Diff line change
@@ -41,6 +41,7 @@ impl From<v1::Argument> for Argument {
required: v1_argument.required.into(),
argument_type: v1_argument.argument_type.into(),
observes: Observes::ReadWrite,
deprecated: None,
}
}
}
8 changes: 8 additions & 0 deletions selene-lib/tests/lints/deprecated/deprecated_params.lua
Original file line number Diff line number Diff line change
@@ -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 {}
17 changes: 17 additions & 0 deletions selene-lib/tests/lints/deprecated/deprecated_params.std.yml
Original file line number Diff line number Diff line change
@@ -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
40 changes: 40 additions & 0 deletions selene-lib/tests/lints/deprecated/deprecated_params.stderr
Original file line number Diff line number Diff line change
@@ -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

2 changes: 2 additions & 0 deletions selene-lib/tests/lints/deprecated/specific_allow.lua
Original file line number Diff line number Diff line change
@@ -5,3 +5,5 @@ more.deprecated_allowed()
wow.extra.deprecated_allowed()

deprecated_allowed.more()

deprecated_param(1)
7 changes: 7 additions & 0 deletions selene-lib/tests/lints/deprecated/specific_allow.std.yml
Original file line number Diff line number Diff line change
@@ -16,3 +16,10 @@ globals:
args: []
deprecated:
message: "this is deprecated"
deprecated_param:
args:
- required: false
type:
display: any
deprecated:
message: this is deprecated
24 changes: 19 additions & 5 deletions selene/src/roblox/generate_std.rs
Original file line number Diff line number Diff line change
@@ -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,

0 comments on commit abf408d

Please sign in to comment.