Skip to content

Commit

Permalink
[move][move-linter] implement public mut context rules
Browse files Browse the repository at this point in the history
  • Loading branch information
tx-tomcat committed May 31, 2024
1 parent 67d6d0d commit 48721ac
Show file tree
Hide file tree
Showing 12 changed files with 163 additions and 163 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ use crate::{
},
};

use super::{LinterDiagCategory, CONSTANT_NAMING_DIAG_CODE, LINT_WARNING_PREFIX};
use super::{LinterDiagnosticCategory, CONSTANT_NAMING_DIAG_CODE, LINT_WARNING_PREFIX};

/// Diagnostic information for constant naming violations.
const CONSTANT_NAMING_DIAG: DiagnosticInfo = custom(
LINT_WARNING_PREFIX,
Severity::Warning,
LinterDiagCategory::Style as u8,
LinterDiagnosticCategory::Style as u8,
CONSTANT_NAMING_DIAG_CODE,
"constant should follow naming convention",
);
Expand Down
42 changes: 26 additions & 16 deletions external-crates/move/crates/move-compiler/src/linters/mod.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

use move_symbol_pool::Symbol;

use crate::{
command_line::compiler::Visitor, diagnostics::codes::WarningFilter,
linters::constant_naming::ConstantNamingVisitor, typing::visitor::TypingVisitor,
linters::constant_naming::ConstantNamingVisitor,
linters::public_mut_tx_context::RequireMutableTxContext, typing::visitor::TypingVisitor,
};
use move_symbol_pool::Symbol;

pub mod constant_naming;
pub mod public_mut_tx_context;

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum LintLevel {
// No linters
Expand All @@ -32,22 +35,28 @@ pub enum LinterDiagnosticCategory {
pub const ALLOW_ATTR_CATEGORY: &str = "lint";
pub const LINT_WARNING_PREFIX: &str = "Lint ";
pub const CONSTANT_NAMING_FILTER_NAME: &str = "constant_naming";

pub const CONSTANT_NAMING_DIAG_CODE: u8 = 1;

pub enum LinterDiagCategory {
Style,
}
pub const REQUIRE_MUTABLE_TX_CONTEXT_FILTER_NAME: &str = "public_mut_tx_context";
pub const REQUIRE_MUTABLE_TX_CONTEXT_DIAG_CODE: u8 = 14;

pub fn known_filters() -> (Option<Symbol>, Vec<WarningFilter>) {
(
Some(ALLOW_ATTR_CATEGORY.into()),
vec![WarningFilter::code(
Some(LINT_WARNING_PREFIX),
LinterDiagCategory::Style as u8,
CONSTANT_NAMING_DIAG_CODE,
Some(CONSTANT_NAMING_FILTER_NAME),
)],
vec![
WarningFilter::code(
Some(LINT_WARNING_PREFIX),
LinterDiagnosticCategory::Style as u8,
CONSTANT_NAMING_DIAG_CODE,
Some(CONSTANT_NAMING_FILTER_NAME),
),
WarningFilter::code(
Some(LINT_WARNING_PREFIX),
LinterDiagnosticCategory::Suspicious as u8,
REQUIRE_MUTABLE_TX_CONTEXT_DIAG_CODE,
Some(REQUIRE_MUTABLE_TX_CONTEXT_FILTER_NAME),
),
],
)
}

Expand All @@ -56,9 +65,10 @@ pub fn linter_visitors(level: LintLevel) -> Vec<Visitor> {
LintLevel::None => vec![],
LintLevel::Default => vec![],
LintLevel::All => {
vec![constant_naming::ConstantNamingVisitor::visitor(
ConstantNamingVisitor,
)]
vec![
constant_naming::ConstantNamingVisitor::visitor(ConstantNamingVisitor),
public_mut_tx_context::RequireMutableTxContext::visitor(RequireMutableTxContext),
]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

//! Enforces that public functions use `&mut TxContext` instead of `&TxContext` to ensure upgradability.
//! Detects and reports instances where a non-mutable reference to `TxContext` is used in public function signatures.
//! Promotes best practices for future-proofing smart contract code by allowing mutation of the transaction context.
use crate::{
diag,
diagnostics::{
codes::{custom, DiagnosticInfo, Severity},
WarningFilters,
},
expansion::ast::{ModuleIdent, Visibility},
naming::ast::{TypeName_, Type_},
parser::ast::{DatatypeName, FunctionName},
shared::CompilationEnv,
typing::{
ast as T,
visitor::{TypingVisitorConstructor, TypingVisitorContext},
},
};
use move_ir_types::location::Loc;

use super::{LinterDiagnosticCategory, LINT_WARNING_PREFIX, REQUIRE_MUTABLE_TX_CONTEXT_DIAG_CODE};

const REQUIRE_MUTABLE_TX_CONTEXT_DIAG: DiagnosticInfo = custom(
LINT_WARNING_PREFIX,
Severity::Warning,
LinterDiagnosticCategory::Suspicious as u8,
REQUIRE_MUTABLE_TX_CONTEXT_DIAG_CODE,
"",
);

pub struct RequireMutableTxContext;

pub struct Context<'a> {
env: &'a mut CompilationEnv,
}

impl TypingVisitorConstructor for RequireMutableTxContext {
type Context<'a> = Context<'a>;

fn context<'a>(env: &'a mut CompilationEnv, _program: &T::Program) -> Self::Context<'a> {
Context { env }
}
}

impl TypingVisitorContext for Context<'_> {
fn visit_function_custom(
&mut self,
_module: ModuleIdent,
_function_name: FunctionName,
fdef: &mut T::Function,
) -> bool {
if let Visibility::Public(_) = fdef.visibility {
for param in &fdef.signature.parameters {
if let Type_::Ref(false, var_type) = &param.2.value {
if let Type_::Apply(_, type_name, _) = &var_type.value {
if let TypeName_::ModuleType(_, DatatypeName(sp!(_, struct_name))) =
&type_name.value
{
if struct_name.to_string() == "TxContext" {
report_non_mutable_tx_context(self.env, type_name.loc);
}
}
}
}
}
}

false
}
fn add_warning_filter_scope(&mut self, filter: WarningFilters) {
self.env.add_warning_filter_scope(filter)
}

fn pop_warning_filter_scope(&mut self) {
self.env.pop_warning_filter_scope()
}
}

fn report_non_mutable_tx_context(env: &mut CompilationEnv, loc: Loc) {
let diag = diag!(
REQUIRE_MUTABLE_TX_CONTEXT_DIAG,
(loc, "Public functions should take `&mut TxContext` instead of `&TxContext` for better upgradability.")
);
env.add_diag(diag);
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,12 @@ pub fn build_member_map(
tyarg_arity,
field_info,
};
assert!(members.insert(name.value(), M::Datatype(ResolvedDatatype::Struct(Box::new(struct_def)))).is_none())
assert!(members
.insert(
name.value(),
M::Datatype(ResolvedDatatype::Struct(Box::new(struct_def)))
)
.is_none())
}
for (enum_name, edef) in mdef.enums.key_cloned_iter() {
let tyarg_arity = edef.type_parameters.len();
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module 0x42::M {
use sui::tx_context::TxContext;
public fun mint(_ctx: &mut TxContext) {

}
}

module sui::tx_context {
struct TxContext has drop {}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
warning[Lint W00001]: constant should follow naming convention
warning[Lint W04001]: constant should follow naming convention
┌─ tests/linter/incorrect_constant_naming.move:3:5
3 │ const Another_BadName: u64 = 42; // Should trigger a warning
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 'Another_BadName' should be ALL_CAPS. Or for error constants, use PascalCase
= This warning can be suppressed with '#[allow(lint(constant_naming))]' applied to the 'module' or module member ('const', 'fun', or 'struct')

warning[Lint W00001]: constant should follow naming convention
warning[Lint W04001]: constant should follow naming convention
┌─ tests/linter/incorrect_constant_naming.move:4:5
4 │ const JSON_Max_Size: u64 = 1048576;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
warning[Lint W02014]:
┌─ tests/linter/incorrect_public_mut_tx_context.move:3:28
3 │ public fun mint(_ctx: &TxContext) {
│ ^^^^^^^^^ Public functions should take `&mut TxContext` instead of `&TxContext` for better upgradability.
= This warning can be suppressed with '#[allow(lint(public_mut_tx_context))]' applied to the 'module' or module member ('const', 'fun', or 'struct')

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module 0x42::M {
use sui::tx_context::TxContext;
public fun mint(_ctx: &TxContext) {

}
}

module sui::tx_context {
struct TxContext has drop {}
}
Loading

0 comments on commit 48721ac

Please sign in to comment.