Skip to content

feat: add empty module validation #4794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,10 @@
has been improved to avoid performing needless checks.
([Giacomo Cavalieri](https://github.com/giacomocavalieri))

- The compiler now emits a warning when a module contains no public definitions
and prevents publishing packages with empty modules to Hex.
([Vitor Souza](https://github.com/vit0rr))

### Build tool

- `gleam update`, `gleam deps update`, and `gleam deps download` will now print
Expand Down
25 changes: 25 additions & 0 deletions compiler-cli/src/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,31 @@ fn do_build_hex_tarball(paths: &ProjectPaths, config: &mut PackageConfig) -> Res
});
}

// empty_modules is a list of modules that do not export any values or types.
// We do not allow publishing packages that contain empty modules.
let empty_modules: Vec<_> = built
.root_package
.modules
.iter()
.filter(|module| {
built
.module_interfaces
.get(&module.name)
.map(|interface| {
// Check if the module exports any values or types
interface.values.is_empty() && interface.types.is_empty()
})
.unwrap_or(false)
})
.map(|module| module.name.clone())
.collect();

if !empty_modules.is_empty() {
return Err(Error::CannotPublishEmptyModules {
unfinished: empty_modules,
});
}

// TODO: If any of the modules in the package contain a leaked internal type then
// refuse to publish as the package is not yet finished.
// We need to move aliases in to the type system first.
Expand Down
10 changes: 10 additions & 0 deletions compiler-core/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,16 @@ impl TypedDefinition {
}
}

pub fn is_public(&self) -> bool {
match self {
Definition::Function(f) => f.publicity.is_public(),
Definition::TypeAlias(t) => t.publicity.is_public(),
Definition::CustomType(t) => t.publicity.is_public(),
Definition::ModuleConstant(c) => c.publicity.is_public(),
Definition::Import(_) => false,
}
}

pub fn find_node(&self, byte_index: u32) -> Option<Located<'_>> {
match self {
Definition::Function(function) => {
Expand Down
18 changes: 18 additions & 0 deletions compiler-core/src/build/package_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,24 @@ fn analyse(
// Register the types from this module so they can be imported into
// other modules.
let _ = module_types.insert(module.name.clone(), module.ast.type_info.clone());

// Check for empty modules and emit warning
let public_definitions = module
.ast
.definitions
.iter()
.filter(|def| def.is_public())
.count();

// Only emit the empty module warning if the module has no definitions at all.
// Modules with only private definitions already emit their own warnings.
if public_definitions == 0 && module.ast.definitions.is_empty() {
warnings.emit(crate::warning::Warning::EmptyModule {
path: module.input_path.clone(),
name: module.name.clone(),
});
}

// Register the successfully type checked module data so that it can be
// used for code generation and in the language server.
modules.push(module);
Expand Down
21 changes: 21 additions & 0 deletions compiler-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,9 @@ file_names.iter().map(|x| x.as_str()).join(", "))]
)]
CannotPublishLeakedInternalType { unfinished: Vec<EcoString> },

#[error("The modules {unfinished:?} are empty and so cannot be published")]
CannotPublishEmptyModules { unfinished: Vec<EcoString> },

#[error("Publishing packages to reserve names is not permitted")]
HexPackageSquatting,

Expand Down Expand Up @@ -1046,6 +1049,24 @@ Please make sure internal types do not appear in public functions and try again.
location: None,
}],

Error::CannotPublishEmptyModules { unfinished } => vec![Diagnostic {
title: "Cannot publish empty modules".into(),
text: wrap_format!(
"These modules contain no public definitions and cannot be published:

{}

Please add public functions, types, or constants to these modules, or remove them and try again.",
unfinished
.iter()
.map(|name| format!(" - {}", name.as_str()))
.join("\n")
),
level: Level::Error,
hint: None,
location: None,
}],

Error::UnableToFindProjectRoot { path } => {
let text = wrap_format!(
"We were unable to find gleam.toml.
Expand Down
13 changes: 13 additions & 0 deletions compiler-core/src/warning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ pub enum Warning {
DeprecatedEnvironmentVariable {
variable: DeprecatedEnvironmentVariable,
},

EmptyModule {
path: Utf8PathBuf,
name: EcoString,
},
}

#[derive(Debug, Clone, Eq, PartialEq, Copy)]
Expand Down Expand Up @@ -1371,6 +1376,14 @@ The imported value could not be used in this module anyway."
location: None,
}
}

Warning::EmptyModule { path: _, name } => Diagnostic {
title: "Empty module".into(),
text: format!("Module '{name}' contains no public definitions."),
hint: Some("Consider adding public functions, types, or constants, or removing this module.".into()),
level: diagnostic::Level::Warning,
location: None,
},
}
}

Expand Down
7 changes: 7 additions & 0 deletions test-package-compiler/cases/empty_module_warning/gleam.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
name = "empty_module_warning"
version = "1.0.0"
description = "Test package with empty modules"
licences = ["Apache-2.0"]

[dependencies]
gleam_stdlib = ">= 0.44.0 and < 2.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// This is an empty module
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub fn main() {
"This module has public definitions"
}
11 changes: 11 additions & 0 deletions test-package-compiler/src/generated_tests.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
---
source: test-package-compiler/src/generated_tests.rs
expression: "./cases/empty_module_warning"
snapshot_kind: text
---
//// /out/lib/the_package/_gleam_artefacts/empty.cache
<.cache binary>

//// /out/lib/the_package/_gleam_artefacts/empty.cache_meta
<53 byte binary>

//// /out/lib/the_package/_gleam_artefacts/empty.erl
-module(empty).


//// /out/lib/the_package/_gleam_artefacts/main.cache
<.cache binary>

//// /out/lib/the_package/_gleam_artefacts/main.cache_meta
<61 byte binary>

//// /out/lib/the_package/_gleam_artefacts/main.erl
-module(main).
-compile([no_auto_import, nowarn_unused_vars, nowarn_unused_function, nowarn_nomatch, inline]).
-define(FILEPATH, "src/main.gleam").
-export([main/0]).

-file("src/main.gleam", 1).
-spec main() -> binary().
main() ->
<<"This module has public definitions"/utf8>>.


//// /out/lib/the_package/ebin/empty_module_warning.app
{application, empty_module_warning, [
{vsn, "1.0.0"},
{applications, [gleam_stdlib]},
{description, "Test package with empty modules"},
{modules, [empty,
main]},
{registered, []}
]}.


//// Warning
warning: Empty module

Module 'empty' contains no public definitions.
Hint: Consider adding public functions, types, or constants, or removing this module.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: test-package-compiler/src/generated_tests.rs
expression: "./cases/erlang_app_generation"
snapshot_kind: text
---
//// /out/lib/the_package/_gleam_artefacts/main.cache
<.cache binary>
Expand All @@ -26,3 +27,10 @@ expression: "./cases/erlang_app_generation"
{modules, [main]},
{registered, []}
]}.


//// Warning
warning: Empty module

Module 'main' contains no public definitions.
Hint: Consider adding public functions, types, or constants, or removing this module.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: test-package-compiler/src/generated_tests.rs
expression: "./cases/erlang_empty"
snapshot_kind: text
---
//// /out/lib/the_package/_gleam_artefacts/empty.cache
<.cache binary>
Expand All @@ -20,3 +21,10 @@ expression: "./cases/erlang_empty"
{modules, [empty]},
{registered, []}
]}.


//// Warning
warning: Empty module

Module 'empty' contains no public definitions.
Hint: Consider adding public functions, types, or constants, or removing this module.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: test-package-compiler/src/generated_tests.rs
expression: "./cases/javascript_empty"
snapshot_kind: text
---
//// /out/lib/the_package/_gleam_artefacts/empty.cache
<.cache binary>
Expand All @@ -14,3 +15,10 @@ export {}

//// /out/lib/the_package/gleam.mjs
export * from "../prelude.mjs";


//// Warning
warning: Empty module

Module 'empty' contains no public definitions.
Hint: Consider adding public functions, types, or constants, or removing this module.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: test-package-compiler/src/generated_tests.rs
expression: "./cases/not_overwriting_erlang_module"
snapshot_kind: text
---
//// /out/lib/the_package/_gleam_artefacts/[email protected]
<.cache binary>
Expand All @@ -20,3 +21,10 @@ expression: "./cases/not_overwriting_erlang_module"
{modules, [app@code]},
{registered, []}
]}.


//// Warning
warning: Empty module

Module 'app/code' contains no public definitions.
Hint: Consider adding public functions, types, or constants, or removing this module.
Loading