Skip to content

Commit

Permalink
[move] Add deprecation attributes to source language (#17964)
Browse files Browse the repository at this point in the history
## Description 

Adds support for deprecation attributes to the source language:
```
#[deprecated]
<member>

#[deprecated(note = b"<string>")]
<member>
```

where `<member>` can be a module, constant, struct, enum, or function. 

Note that deprecation checking must be performed after typechecking
since we need to have methods resolved before determining whether the
call uses a deprecated function.

A couple things to note:
1. Accessing any member within a deprecated module, will raise a
deprecation warning. The one exception to this is access/calls within a
deprecated module -- in which case no warnings will be raised.

```move
#[deprecated]
module 0x42::m {
   public fun f() { }
   public fun call_f() { f(); } // no warning raised since internal access
}
```

```move
#[deprecated]
module 0x42::m {
   public fun f() { }
   public fun call_f() { f(); } // no warning raised since internal access
}

module 0x42::k {

 fun caller() { 0x42::m::f(); } // Will raise an error since an external access
}
```


2. Specific deprecations always override module deprecations, and they
will always raise a deprecation warning, even for internal accesses. In
particular:

```move
#[deprecated]
module 0x42::m {
   [deprecated(note = b"note")]
   public fun f() { }
   public fun call_f() { f(); } // warning will be raised with note "note"
}

module 0x42::k {
    fun caller() { 0x42::m::f(); } // warning will be raised with note "note"
}
```

3. Deprecations are grouped into single diagnostics -- one diagnostic
per `(deprecation, named_context)` pair where a named context can be
either a struct, enum, constant, or function. E.g., for the following
code
```move
module 0x42::m {
    #[deprecated]
    const A: u64 = 1;

    #[deprecated(note = b"use D instead")]
    const B: u64 = 2;

    #[deprecated(note = b"You should use E instead")]
    const C: u64 = 3;

    const D: u64 = 4;
    const E: u64 = 5;

    const Combo: u64 = {
        A + B + C + D + E + B
    };
}
```

Will produce the following warnings since:
```
warning[W04037]: use of deprecated constant
   ┌─ tests/move_2024/typing/deprecation_use_in_constants.move:15:9
   │
15 │         A + B + C + D + E + B
   │         ^ This constant is deprecated

warning[W04037]: use of deprecated constant
   ┌─ tests/move_2024/typing/deprecation_use_in_constants.move:15:13
   │
15 │         A + B + C + D + E + B
   │             ^               - Deprecated constant used again here
   │             │                
   │             This constant is deprecated: use D instead

warning[W04037]: use of deprecated constant
   ┌─ tests/move_2024/typing/deprecation_use_in_constants.move:15:17
   │
15 │         A + B + C + D + E + B
   │                 ^ This constant is deprecated: You should use E instead
```

Notice the warning for `B` has multiple labels -- one per usage of the
deprecated constant within the named context.

## Test plan 

Added new positive and negative tests for the deprecation behavior.
  • Loading branch information
tzakian authored Jun 28, 2024
1 parent be9c2c1 commit adf395f
Show file tree
Hide file tree
Showing 34 changed files with 1,336 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "A"
edition = "2024.beta"

[addresses]
A = "0x1"

[dependencies]
Dep = { local = "./dep" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
Command `build -v`:
INCLUDING DEPENDENCY Dep
BUILDING A
warning[W04037]: deprecated usage
┌─ ./sources/l.move:8:13
8 │ am::deprecated_function();
│ ^^^^^^^^^^^^^^^^^^^ The function 'A::m::deprecated_function' is deprecated: use a different function instead

warning[W04037]: deprecated usage
┌─ ./sources/l.move:10:25
10 │ mod_deprecated::deprecated_function();
│ ^^^^^^^^^^^^^^^^^^^ The function 'A::mod_deprecated::deprecated_function' is deprecated: This function is deprecated with a deprecated module

warning[W04037]: deprecated usage
┌─ ./sources/l.move:11:25
11 │ mod_deprecated::make_f();
│ ^^^^^^ The 'A::mod_deprecated::make_f' member of the module 'A::mod_deprecated' is deprecated. It is deprecated since its whole module is marked deprecated: This module is deprecated

warning[W04037]: deprecated usage
┌─ ./sources/l.move:13:15
13 │ l<am::Bar>();
│ ^^^ The struct 'A::m::Bar' is deprecated: use a different struct instead

warning[W04037]: deprecated usage
┌─ ./sources/l.move:15:27
15 │ l<mod_deprecated::F>();
│ ^ The 'A::mod_deprecated::F' member of the module 'A::mod_deprecated' is deprecated. It is deprecated since its whole module is marked deprecated: This module is deprecated

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
build -v
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "Dep"
edition = "2024.beta"

[addresses]
A = "_"
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module A::m {
#[deprecated(note = b"use a different struct instead")]
public struct Bar() has drop;


public fun make_bar(): Bar {
Bar()
}


#[deprecated(note = b"use a different function instead")]
public fun deprecated_function() { }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#[deprecated(note = b"This module is deprecated")]
module A::mod_deprecated {
public struct F() has drop;

public fun make_f(): F {
F()
}

#[deprecated(note = b"This function is deprecated with a deprecated module")]
public fun deprecated_function() { }
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module A::l {
use A::m as am;
use A::mod_deprecated;

#[allow(unused_function)]
fun f() {
am::make_bar();
am::deprecated_function();

mod_deprecated::deprecated_function();
mod_deprecated::make_f();

l<am::Bar>();

l<mod_deprecated::F>();
}

#[allow(unused_function, unused_type_parameter)]
fun l<T>() { }
}
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ codes!(
IncompatibleSyntaxMethods: { msg: "'syntax' method types differ", severity: BlockingError },
InvalidErrorUsage: { msg: "invalid constant usage in error context", severity: BlockingError },
IncompletePattern: { msg: "non-exhaustive pattern", severity: BlockingError },
DeprecatedUsage: { msg: "deprecated usage", severity: Warning },
],
// errors for ability rules. mostly typing/translate
AbilitySafety: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,8 @@ fn gate_known_attribute(context: &mut Context, loc: Loc, known: &KnownAttribute)
| KnownAttribute::Diagnostic(_)
| KnownAttribute::DefinesPrimitive(_)
| KnownAttribute::External(_)
| KnownAttribute::Syntax(_) => (),
| KnownAttribute::Syntax(_)
| KnownAttribute::Deprecation(_) => (),
KnownAttribute::Error(_) => {
let pkg = context.current_package();
context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub enum KnownAttribute {
External(ExternalAttribute),
Syntax(SyntaxAttribute),
Error(ErrorAttribute),
Deprecation(DeprecationAttribute),
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
Expand Down Expand Up @@ -74,6 +75,9 @@ pub struct ExternalAttribute;
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub struct ErrorAttribute;

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub struct DeprecationAttribute;

impl AttributePosition {
const ALL: &'static [Self] = &[
Self::AddressBlock,
Expand Down Expand Up @@ -102,6 +106,7 @@ impl KnownAttribute {
ExternalAttribute::EXTERNAL => ExternalAttribute.into(),
SyntaxAttribute::SYNTAX => SyntaxAttribute::Syntax.into(),
ErrorAttribute::ERROR => ErrorAttribute.into(),
DeprecationAttribute::DEPRECATED => DeprecationAttribute.into(),
_ => return None,
})
}
Expand All @@ -116,6 +121,7 @@ impl KnownAttribute {
Self::External(a) => a.name(),
Self::Syntax(a) => a.name(),
Self::Error(a) => a.name(),
Self::Deprecation(a) => a.name(),
}
}

Expand All @@ -129,6 +135,7 @@ impl KnownAttribute {
Self::External(a) => a.expected_positions(),
Self::Syntax(a) => a.expected_positions(),
Self::Error(a) => a.expected_positions(),
Self::Deprecation(a) => a.expected_positions(),
}
}
}
Expand Down Expand Up @@ -326,6 +333,27 @@ impl ErrorAttribute {
}
}

impl DeprecationAttribute {
pub const DEPRECATED: &'static str = "deprecated";

pub const fn name(&self) -> &str {
Self::DEPRECATED
}

pub fn expected_positions(&self) -> &'static BTreeSet<AttributePosition> {
static DEPRECATION_POSITIONS: Lazy<BTreeSet<AttributePosition>> = Lazy::new(|| {
BTreeSet::from([
AttributePosition::Constant,
AttributePosition::Module,
AttributePosition::Struct,
AttributePosition::Enum,
AttributePosition::Function,
])
});
&DEPRECATION_POSITIONS
}
}

//**************************************************************************************************
// Display
//**************************************************************************************************
Expand Down Expand Up @@ -357,6 +385,7 @@ impl fmt::Display for KnownAttribute {
Self::External(a) => a.fmt(f),
Self::Syntax(a) => a.fmt(f),
Self::Error(a) => a.fmt(f),
Self::Deprecation(a) => a.fmt(f),
}
}
}
Expand Down Expand Up @@ -409,6 +438,12 @@ impl fmt::Display for ErrorAttribute {
}
}

impl fmt::Display for DeprecationAttribute {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.name())
}
}

//**************************************************************************************************
// From
//**************************************************************************************************
Expand Down Expand Up @@ -453,3 +488,8 @@ impl From<ErrorAttribute> for KnownAttribute {
Self::Error(a)
}
}
impl From<DeprecationAttribute> for KnownAttribute {
fn from(a: DeprecationAttribute) -> Self {
Self::Deprecation(a)
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

use std::{collections::BTreeMap, sync::Arc};
use std::{collections::BTreeMap, fmt::Display, sync::Arc};

use move_ir_types::location::Loc;
use move_symbol_pool::Symbol;
Expand All @@ -19,6 +19,8 @@ use crate::{
FullyCompiledProgram,
};

use self::known_attributes::AttributePosition;

#[derive(Debug, Clone)]
pub struct FunctionInfo {
pub attributes: Attributes,
Expand Down Expand Up @@ -56,6 +58,14 @@ pub enum DatatypeKind {
Enum,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum NamedMemberKind {
Struct,
Enum,
Function,
Constant,
}

#[derive(Debug, Clone)]
pub struct ProgramInfo<const AFTER_TYPING: bool> {
pub modules: UniqueMap<ModuleIdent, ModuleInfo>,
Expand Down Expand Up @@ -230,14 +240,26 @@ impl<const AFTER_TYPING: bool> ProgramInfo<AFTER_TYPING> {
&self.enum_definition(m, n).type_parameters
}

pub fn named_member_kind(&self, m: ModuleIdent, n: Name) -> NamedMemberKind {
let minfo = self.module(&m);
if minfo.structs.contains_key(&DatatypeName(n)) {
NamedMemberKind::Struct
} else if minfo.enums.contains_key(&DatatypeName(n)) {
NamedMemberKind::Enum
} else if minfo.functions.contains_key(&FunctionName(n)) {
NamedMemberKind::Function
} else if minfo.constants.contains_key(&ConstantName(n)) {
NamedMemberKind::Constant
} else {
panic!("ICE should have failed in naming")
}
}

pub fn datatype_kind(&self, m: &ModuleIdent, n: &DatatypeName) -> DatatypeKind {
match (
self.module(m).structs.contains_key(n),
self.module(m).enums.contains_key(n),
) {
(true, false) => DatatypeKind::Struct,
(false, true) => DatatypeKind::Enum,
(false, false) | (true, true) => panic!("ICE should have failed in naming"),
match self.named_member_kind(*m, n.0) {
NamedMemberKind::Struct => DatatypeKind::Struct,
NamedMemberKind::Enum => DatatypeKind::Enum,
_ => panic!("ICE should have failed in naming"),
}
}

Expand Down Expand Up @@ -352,3 +374,34 @@ impl<const AFTER_TYPING: bool> ProgramInfo<AFTER_TYPING> {
}
}
}

impl Display for NamedMemberKind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
NamedMemberKind::Struct => write!(f, "struct"),
NamedMemberKind::Enum => write!(f, "enum"),
NamedMemberKind::Function => write!(f, "function"),
NamedMemberKind::Constant => write!(f, "constant"),
}
}
}

impl From<NamedMemberKind> for AttributePosition {
fn from(nmk: NamedMemberKind) -> Self {
match nmk {
NamedMemberKind::Struct => AttributePosition::Struct,
NamedMemberKind::Enum => AttributePosition::Enum,
NamedMemberKind::Function => AttributePosition::Function,
NamedMemberKind::Constant => AttributePosition::Constant,
}
}
}

impl From<DatatypeKind> for NamedMemberKind {
fn from(dt: DatatypeKind) -> Self {
match dt {
DatatypeKind::Struct => NamedMemberKind::Struct,
DatatypeKind::Enum => NamedMemberKind::Enum,
}
}
}
Loading

0 comments on commit adf395f

Please sign in to comment.