Skip to content

Commit

Permalink
Implement module privacy rules (FuelLabs#4474)
Browse files Browse the repository at this point in the history
## Description

This change mainly adds checks to enforce the new module privacy rules
and supporting changes for it.

Changes include updating std and core to use
public modules, updating the parser to allow the use of the `pub mod`
syntax and adding an error type for private modules.

This change is implemented behind a `--experimental-private-modules`
experimental flag and not enabled by default.

It implements part of FuelLabs#4446, the
`pub use` syntax is yet to be implemented.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

Co-authored-by: Joshua Batty <[email protected]>
  • Loading branch information
IGI-111 and JoshuaBatty authored Apr 28, 2023
1 parent af5f536 commit 1ecc5e7
Showing 61 changed files with 647 additions and 147 deletions.
3 changes: 3 additions & 0 deletions forc-pkg/src/manifest.rs
Original file line number Diff line number Diff line change
@@ -214,6 +214,7 @@ pub struct BuildProfile {
pub include_tests: bool,
pub json_abi_with_callpaths: bool,
pub error_on_warnings: bool,
pub experimental_private_modules: bool,
}

impl Dependency {
@@ -657,6 +658,7 @@ impl BuildProfile {
include_tests: false,
json_abi_with_callpaths: false,
error_on_warnings: false,
experimental_private_modules: false,
}
}

@@ -673,6 +675,7 @@ impl BuildProfile {
include_tests: false,
json_abi_with_callpaths: false,
error_on_warnings: false,
experimental_private_modules: false,
}
}
}
18 changes: 16 additions & 2 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
@@ -39,7 +39,7 @@ use sway_core::{
language::{
lexed::LexedProgram,
parsed::{ParseProgram, TreeType},
ty,
ty, Visibility,
},
semantic_analysis::namespace,
source_map::SourceMap,
@@ -302,6 +302,8 @@ pub struct BuildOpts {
pub tests: bool,
/// The set of options to filter by member project kind.
pub member_filter: MemberFilter,
/// Enable the experimental module privacy enforcement.
pub experimental_private_modules: bool,
}

/// The set of options to filter type of projects to build in a workspace.
@@ -1544,7 +1546,8 @@ pub fn sway_build_config(
.print_finalized_asm(build_profile.print_finalized_asm)
.print_intermediate_asm(build_profile.print_intermediate_asm)
.print_ir(build_profile.print_ir)
.include_tests(build_profile.include_tests);
.include_tests(build_profile.include_tests)
.experimental_private_modules(build_profile.experimental_private_modules);
Ok(build_config)
}

@@ -1570,6 +1573,7 @@ pub fn dependency_namespace(
node: NodeIx,
engines: Engines<'_>,
contract_id_value: Option<ContractIdConst>,
experimental_private_modules: bool,
) -> Result<namespace::Module, vec1::Vec1<CompileError>> {
// TODO: Clean this up when config-time constants v1 are removed.
let node_idx = &graph[node];
@@ -1582,6 +1586,7 @@ pub fn dependency_namespace(

namespace.is_external = true;
namespace.name = name;
namespace.visibility = Visibility::Public;

// Add direct dependencies.
let mut core_added = false;
@@ -1611,6 +1616,7 @@ pub fn dependency_namespace(
)?;
ns.is_external = true;
ns.name = name;
ns.visibility = Visibility::Public;
ns
}
};
@@ -1633,13 +1639,15 @@ pub fn dependency_namespace(
&[CORE, PRELUDE].map(|s| Ident::new_no_span(s.into())),
&[],
engines,
experimental_private_modules,
);

if has_std_dep(graph, node) {
namespace.star_import_with_reexports(
&[STD, PRELUDE].map(|s| Ident::new_no_span(s.into())),
&[],
engines,
experimental_private_modules,
);
}

@@ -1986,6 +1994,7 @@ fn build_profile_from_opts(
time_phases,
tests,
error_on_warnings,
experimental_private_modules,
..
} = build_options;
let mut selected_build_profile = BuildProfile::DEBUG;
@@ -2036,6 +2045,7 @@ fn build_profile_from_opts(
profile.include_tests |= tests;
profile.json_abi_with_callpaths |= pkg.json_abi_with_callpaths;
profile.error_on_warnings |= error_on_warnings;
profile.experimental_private_modules |= experimental_private_modules;

Ok((selected_build_profile.to_string(), profile))
}
@@ -2268,6 +2278,7 @@ pub fn build(
node,
engines,
None,
profile.experimental_private_modules,
) {
Ok(o) => o,
Err(errs) => return fail(&[], &errs),
@@ -2323,6 +2334,7 @@ pub fn build(
node,
engines,
contract_id_value.clone(),
profile.experimental_private_modules,
) {
Ok(o) => o,
Err(errs) => return fail(&[], &errs),
@@ -2506,6 +2518,7 @@ pub fn check(
terse_mode: bool,
include_tests: bool,
engines: Engines<'_>,
experimental_private_modules: bool,
) -> anyhow::Result<Vec<CompileResult<Programs>>> {
let mut lib_namespace_map = Default::default();
let mut source_map = SourceMap::new();
@@ -2523,6 +2536,7 @@ pub fn check(
node,
engines,
None,
experimental_private_modules,
)
.expect("failed to create dependency namespace");

1 change: 1 addition & 0 deletions forc-plugins/forc-client/src/op/deploy.rs
Original file line number Diff line number Diff line change
@@ -255,6 +255,7 @@ fn build_opts_from_cmd(cmd: &cmd::Deploy) -> pkg::BuildOpts {
build_target: BuildTarget::default(),
tests: false,
member_filter: pkg::MemberFilter::only_contracts(),
experimental_private_modules: cmd.build_profile.experimental_private_modules,
}
}

1 change: 1 addition & 0 deletions forc-plugins/forc-client/src/op/run.rs
Original file line number Diff line number Diff line change
@@ -176,5 +176,6 @@ fn build_opts_from_cmd(cmd: &cmd::Run) -> pkg::BuildOpts {
debug_outfile: cmd.build_output.debug_file.clone(),
tests: false,
member_filter: pkg::MemberFilter::only_scripts(),
experimental_private_modules: cmd.build_profile.experimental_private_modules,
}
}
1 change: 1 addition & 0 deletions forc-plugins/forc-doc/src/main.rs
Original file line number Diff line number Diff line change
@@ -88,6 +88,7 @@ pub fn main() -> Result<()> {
silent,
tests_enabled,
engines,
true,
)?
.pop()
.and_then(|compilation| compilation.value)
3 changes: 3 additions & 0 deletions forc-test/src/lib.rs
Original file line number Diff line number Diff line change
@@ -151,6 +151,8 @@ pub struct Opts {
pub error_on_warnings: bool,
/// Output the time elapsed over each part of the compilation process.
pub time_phases: bool,
/// Enable the experimental module privacy enforcement.
pub experimental_private_modules: bool,
}

/// The set of options provided for controlling logs printed for each test.
@@ -480,6 +482,7 @@ impl Opts {
time_phases: self.time_phases,
tests: true,
member_filter: Default::default(),
experimental_private_modules: self.experimental_private_modules,
}
}
}
3 changes: 3 additions & 0 deletions forc/src/cli/commands/check.rs
Original file line number Diff line number Diff line change
@@ -29,6 +29,9 @@ pub struct Command {
/// Disable checking unit tests.
#[clap(long = "disable-tests")]
pub disable_tests: bool,
/// Enable the experimental module privacy enforcement.
#[clap(long)]
pub experimental_private_modules: bool,
}

pub(crate) fn exec(command: Command) -> ForcResult<()> {
1 change: 1 addition & 0 deletions forc/src/cli/commands/test.rs
Original file line number Diff line number Diff line change
@@ -199,5 +199,6 @@ fn opts_from_cmd(cmd: Command) -> forc_test::Opts {
binary_outfile: cmd.build.output.bin_file,
debug_outfile: cmd.build.output.debug_file,
build_target: cmd.build.build_target,
experimental_private_modules: cmd.build.profile.experimental_private_modules,
}
}
2 changes: 2 additions & 0 deletions forc/src/cli/shared.rs
Original file line number Diff line number Diff line change
@@ -49,6 +49,8 @@ pub struct BuildProfile {
/// Treat warnings as errors.
#[clap(long)]
pub error_on_warnings: bool,
#[clap(long)]
pub experimental_private_modules: bool,
}

/// Options related to printing stages of compiler output.
1 change: 1 addition & 0 deletions forc/src/ops/forc_build.rs
Original file line number Diff line number Diff line change
@@ -39,5 +39,6 @@ fn opts_from_cmd(cmd: BuildCommand) -> pkg::BuildOpts {
build_target: cmd.build.build_target,
tests: cmd.tests,
member_filter: Default::default(),
experimental_private_modules: cmd.build.profile.experimental_private_modules,
}
}
10 changes: 9 additions & 1 deletion forc/src/ops/forc_check.rs
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@ pub fn check(command: CheckCommand, engines: Engines<'_>) -> Result<CompileResul
terse_mode,
locked,
disable_tests,
experimental_private_modules,
} = command;

let this_dir = if let Some(ref path) = path {
@@ -27,7 +28,14 @@ pub fn check(command: CheckCommand, engines: Engines<'_>) -> Result<CompileResul
pkg::BuildPlan::from_lock_and_manifests(&lock_path, &member_manifests, locked, offline)?;
let tests_enabled = !disable_tests;

let mut v = pkg::check(&plan, build_target, terse_mode, tests_enabled, engines)?;
let mut v = pkg::check(
&plan,
build_target,
terse_mode,
tests_enabled,
engines,
experimental_private_modules,
)?;
let res = v
.pop()
.expect("there is guaranteed to be at least one elem in the vector")
1 change: 1 addition & 0 deletions forc/src/ops/forc_contract_id.rs
Original file line number Diff line number Diff line change
@@ -75,5 +75,6 @@ fn build_opts_from_cmd(cmd: &ContractIdCommand) -> pkg::BuildOpts {
build_target: BuildTarget::default(),
tests: false,
member_filter: pkg::MemberFilter::only_contracts(),
experimental_private_modules: cmd.build_profile.experimental_private_modules,
}
}
1 change: 1 addition & 0 deletions forc/src/ops/forc_predicate_root.rs
Original file line number Diff line number Diff line change
@@ -43,5 +43,6 @@ fn build_opts_from_cmd(cmd: PredicateRootCommand) -> pkg::BuildOpts {
build_target: BuildTarget::default(),
tests: false,
member_filter: pkg::MemberFilter::only_predicates(),
experimental_private_modules: cmd.build_profile.experimental_private_modules,
}
}
1 change: 1 addition & 0 deletions sway-ast/src/submodule.rs
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ pub struct Submodule {
pub mod_token: ModToken,
pub name: Ident,
pub semicolon_token: SemicolonToken,
pub visibility: Option<PubToken>,
}

impl Spanned for Submodule {
9 changes: 9 additions & 0 deletions sway-core/src/build_config.rs
Original file line number Diff line number Diff line change
@@ -46,6 +46,7 @@ pub struct BuildConfig {
pub(crate) print_finalized_asm: bool,
pub(crate) print_ir: bool,
pub(crate) include_tests: bool,
pub(crate) experimental_private_modules: bool,
}

impl BuildConfig {
@@ -88,6 +89,7 @@ impl BuildConfig {
print_finalized_asm: false,
print_ir: false,
include_tests: false,
experimental_private_modules: false,
}
}

@@ -126,6 +128,13 @@ impl BuildConfig {
}
}

pub fn experimental_private_modules(self, a: bool) -> Self {
Self {
experimental_private_modules: a,
..self
}
}

/// Whether or not to include test functions in parsing, type-checking and codegen.
///
/// This should be set to `true` by invocations like `forc test` or `forc check --tests`.
6 changes: 5 additions & 1 deletion sway-core/src/language/parsed/module.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::{language::ModName, transform};
use crate::{
language::{ModName, Visibility},
transform,
};

use super::ParseTree;
use sway_types::Span;
@@ -22,4 +25,5 @@ pub struct ParseModule {
pub struct ParseSubmodule {
pub module: ParseModule,
pub mod_name_span: Span,
pub visibility: Visibility,
}
26 changes: 23 additions & 3 deletions sway-core/src/lib.rs
Original file line number Diff line number Diff line change
@@ -48,11 +48,11 @@ pub mod types;

pub use error::CompileResult;
use sway_error::error::CompileError;
use sway_error::warning::CompileWarning;
use sway_error::warning::{CompileWarning, Warning};
use sway_types::{ident::Ident, span, Spanned};
pub use type_system::*;

use language::{lexed, parsed, ty};
use language::{lexed, parsed, ty, Visibility};
use transform::to_parsed_lang::{self, convert_module_kind};

pub mod fuel_prelude {
@@ -244,6 +244,10 @@ fn parse_submodules(

let parse_submodule = parsed::ParseSubmodule {
module: parse_module,
visibility: match submod.visibility {
Some(..) => Visibility::Public,
None => Visibility::Private,
},
mod_name_span: submod.name.span(),
};
let lexed_submodule = lexed::LexedSubmodule {
@@ -334,12 +338,28 @@ pub fn parsed_to_ast(
build_config: Option<&BuildConfig>,
package_name: &str,
) -> CompileResult<ty::TyProgram> {
let experimental_private_modules =
build_config.map_or(true, |b| b.experimental_private_modules);
// Type check the program.
let CompileResult {
value: typed_program_opt,
mut warnings,
mut errors,
} = ty::TyProgram::type_check(engines, parse_program, initial_namespace, package_name);
} = ty::TyProgram::type_check(
engines,
parse_program,
initial_namespace,
package_name,
experimental_private_modules,
);

if !experimental_private_modules {
warnings.push(CompileWarning {
span: parse_program.root.span.clone(),
warning_content: Warning::ModulePrivacyDisabled,
})
}

let mut typed_program = match typed_program_opt {
Some(typed_program) => typed_program,
None => return err(warnings, errors),
Original file line number Diff line number Diff line change
@@ -158,7 +158,7 @@ impl ty::TyTraitDecl {
let mut new_items = vec![];
for method in methods.into_iter() {
let method = check!(
ty::TyFunctionDecl::type_check(ctx.by_ref(), method.clone(), true, false),
ty::TyFunctionDecl::type_check(ctx.by_ref(), method.clone(), true, false,),
ty::TyFunctionDecl::error(method),
warnings,
errors
Original file line number Diff line number Diff line change
@@ -1353,6 +1353,7 @@ impl ty::TyExpression {
&suffix,
ctx.self_type(),
ctx.engines(),
ctx.experimental_private_modules_enabled()
),
return None,
const_probe_warnings,
Original file line number Diff line number Diff line change
@@ -440,6 +440,7 @@ pub(crate) fn resolve_method_name(
ctx.self_type(),
&arguments,
engines,
ctx.experimental_private_modules_enabled()
),
return err(warnings, errors),
warnings,
@@ -467,6 +468,7 @@ pub(crate) fn resolve_method_name(
ctx.self_type(),
&arguments,
engines,
ctx.experimental_private_modules_enabled(),
),
return err(warnings, errors),
warnings,
@@ -494,6 +496,7 @@ pub(crate) fn resolve_method_name(
ctx.self_type(),
&arguments,
engines,
ctx.experimental_private_modules_enabled(),
),
return err(warnings, errors),
warnings,
Loading

0 comments on commit 1ecc5e7

Please sign in to comment.