Skip to content
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@
```
([fruno](https://github.com/fruno-bulax/))

- Gleam will now warn when running a deprecated or internal module main function
([Kevin Chen](https://github.com/kfc35) & [Lioncat2002](https://github.com/Lioncat2002))

### Build tool

- The help text displayed by `gleam dev --help`, `gleam test --help`, and
Expand Down
10 changes: 6 additions & 4 deletions compiler-cli/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use gleam_core::{
build::{Built, Codegen, NullTelemetry, Options, ProjectCompiler, Telemetry},
manifest::Manifest,
paths::ProjectPaths,
warning::WarningEmitterIO,
warning::WarningEmitter,
};

use crate::{
Expand All @@ -28,14 +28,16 @@ pub fn download_dependencies(paths: &ProjectPaths, telemetry: impl Telemetry) ->
}

pub fn main(paths: &ProjectPaths, options: Options, manifest: Manifest) -> Result<Built> {
main_with_warnings(paths, options, manifest, Rc::new(ConsoleWarningEmitter))
let console_warning_emitter = Rc::new(ConsoleWarningEmitter);
let warnings = Rc::new(WarningEmitter::new(console_warning_emitter));
main_with_warnings(paths, options, manifest, warnings)
}

pub(crate) fn main_with_warnings(
paths: &ProjectPaths,
options: Options,
manifest: Manifest,
warnings: Rc<dyn WarningEmitterIO>,
warnings: Rc<WarningEmitter>,
) -> Result<Built> {
let perform_codegen = options.codegen;
let root_config = crate::config::root_config(paths)?;
Expand All @@ -55,7 +57,7 @@ pub(crate) fn main_with_warnings(
tracing::info!("Compiling packages");
let result = {
let _guard = lock.lock(telemetry);
let compiler = ProjectCompiler::new(
let compiler = ProjectCompiler::new_with_warning_emitter(
root_config,
options,
manifest.packages,
Expand Down
4 changes: 2 additions & 2 deletions compiler-cli/src/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use gleam_core::{
error::{FileIoAction, FileKind},
paths::ProjectPaths,
type_,
warning::VectorWarningEmitterIO,
warning::{VectorWarningEmitterIO, WarningEmitter},
};
use hexpm::version::Version;

Expand All @@ -30,7 +30,7 @@ pub fn run(paths: &ProjectPaths) -> Result<()> {
no_print_progress: false,
},
build::download_dependencies(paths, cli::Reporter::new())?,
warnings.clone(),
Rc::new(WarningEmitter::new(warnings.clone())),
)?;
let warnings = warnings.take();

Expand Down
64 changes: 55 additions & 9 deletions compiler-cli/src/run.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
use std::sync::OnceLock;
use std::{rc::Rc, sync::OnceLock};

use camino::Utf8PathBuf;
use ecow::EcoString;
use gleam_core::{
Warning,
analyse::TargetSupport,
build::{Built, Codegen, Compile, Mode, NullTelemetry, Options, Runtime, Target, Telemetry},
config::{DenoFlag, PackageConfig},
error::Error,
io::{Command, CommandExecutor, Stdio},
paths::ProjectPaths,
type_::ModuleFunction,
warning::{WarningEmitter, WarningEmitterIO},
};

use crate::{config::PackageKind, fs::ProjectIO};
use crate::{
config::PackageKind,
fs::{ConsoleWarningEmitter, ProjectIO},
};

#[derive(Debug, Clone, Copy)]
pub enum Which {
Expand Down Expand Up @@ -78,7 +83,7 @@ pub fn setup(
};

// Get the config for the module that is being run to check the target.
// Also get the kind of the package the module belongs to: wether the module
// Also get the kind of the package the module belongs to: whether the module
// belongs to a dependency or to the root package.
let (mod_config, package_kind) = match &module {
Some(mod_path) => {
Expand All @@ -91,11 +96,13 @@ pub fn setup(
let root_config = crate::config::root_config(paths)?;

// Determine which module to run
let module = module.unwrap_or(match which {
Which::Src => root_config.name.to_string(),
Which::Test => format!("{}_test", &root_config.name),
Which::Dev => format!("{}_dev", &root_config.name),
});
let module: EcoString = module
.unwrap_or(match which {
Which::Src => root_config.name.to_string(),
Which::Test => format!("{}_test", &root_config.name),
Which::Dev => format!("{}_dev", &root_config.name),
})
.into();

let target = target.unwrap_or(mod_config.target);

Expand All @@ -122,11 +129,50 @@ pub fn setup(
no_print_progress,
};

let built = crate::build::main(paths, options, manifest)?;
let console_warning_emitter = Rc::new(ConsoleWarningEmitter);
let warning_emitter_with_counter =
Rc::new(WarningEmitter::new(console_warning_emitter.clone()));
let built = crate::build::main_with_warnings(
paths,
options,
manifest,
warning_emitter_with_counter.clone(),
)?;

// Warn if the module being run is internal and NOT in the root package
match package_kind {
PackageKind::Root => {}
PackageKind::Dependency => {
if mod_config.is_internal_module(&module) {
let warning = Warning::InternalModuleMainFunction {
module: module.clone(),
};
console_warning_emitter.emit_warning(warning);
}
}
}

// A module can not be run if it does not exist or does not have a public main function.
let main_function = get_or_suggest_main_function(built, &module, target)?;

// Warn if the main function being run has been deprecated
match main_function.deprecation {
gleam_core::type_::Deprecation::Deprecated { message } => {
let warning = Warning::DeprecatedMainFunction {
module: module.clone(),
message,
};
// warning_emitter.emit also increments a counter. However, we only want to
// increment that counter for root package warnings. For dependencies,
// we just print the warning directly to console.
match package_kind {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just highlighting this section to see if this logic is correct re: warning emitter usage

PackageKind::Root => warning_emitter_with_counter.emit(warning),
PackageKind::Dependency => console_warning_emitter.emit_warning(warning),
}
}
gleam_core::type_::Deprecation::NotDeprecated => {}
}

telemetry.running(&format!("{module}.main"));

// Get the command to run the project.
Expand Down
24 changes: 22 additions & 2 deletions compiler-core/src/build/project_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub struct ProjectCompiler<IO> {
/// The set of modules that have had partial compilation done since the last
/// successful compilation.
incomplete_modules: HashSet<EcoString>,
warnings: WarningEmitter,
warnings: Rc<WarningEmitter>,
telemetry: &'static dyn Telemetry,
options: Options,
paths: ProjectPaths,
Expand All @@ -135,6 +135,26 @@ where
warning_emitter: Rc<dyn WarningEmitterIO>,
paths: ProjectPaths,
io: IO,
) -> Self {
Self::new_with_warning_emitter(
config,
options,
packages,
telemetry,
Rc::new(WarningEmitter::new(warning_emitter)),
paths,
io,
)
}

pub fn new_with_warning_emitter(
config: PackageConfig,
options: Options,
packages: Vec<ManifestPackage>,
telemetry: &'static dyn Telemetry,
warning_emitter: Rc<WarningEmitter>,
paths: ProjectPaths,
io: IO,
) -> Self {
let packages = packages
.into_iter()
Expand All @@ -147,7 +167,7 @@ where
stale_modules: StaleTracker::default(),
incomplete_modules: HashSet::new(),
ids: UniqueIdGenerator::new(),
warnings: WarningEmitter::new(warning_emitter),
warnings: warning_emitter,
subprocess_stdio: Stdio::Inherit,
telemetry,
packages,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: compiler-core/src/warning.rs
expression: warning.to_pretty_string()
---
warning: Deprecated main function

The main function in module `apple` was deprecated with this message: Use
some other method please!
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: compiler-core/src/warning.rs
expression: warning.to_pretty_string()
---
warning: Main function in internal module

The main function is in internal module `internal/picking`. It is not
recommended for public use.
2 changes: 2 additions & 0 deletions compiler-core/src/type_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,7 @@ impl ModuleValueConstructor {
#[derive(Debug, Clone)]
pub struct ModuleFunction {
pub package: EcoString,
pub deprecation: Deprecation,
}

#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -1134,6 +1135,7 @@ impl ModuleInterface {

Ok(ModuleFunction {
package: self.package.clone(),
deprecation: value.deprecation.clone(),
})
}

Expand Down
52 changes: 52 additions & 0 deletions compiler-core/src/warning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,13 @@ pub enum Warning {
src: EcoString,
location: SrcSpan,
},
InternalModuleMainFunction {
module: EcoString,
},
DeprecatedMainFunction {
module: EcoString,
message: EcoString,
},
}

#[derive(Debug, Clone, Eq, PartialEq, Copy)]
Expand Down Expand Up @@ -275,6 +282,32 @@ pub enum DeprecatedSyntaxWarning {
impl Warning {
pub fn to_diagnostic(&self) -> Diagnostic {
match self {
Warning::DeprecatedMainFunction { module, message } => {
let text = wrap(&format!(
"The main function in module `{module}` \
was deprecated with this message: {message}"
));
Diagnostic {
title: "Deprecated main function".into(),
text,
level: diagnostic::Level::Warning,
location: None,
hint: None,
}
}
Warning::InternalModuleMainFunction { module } => {
let text = wrap(&format!(
"The main function is in internal module `{module}`. \
It is not recommended for public use."
));
Diagnostic {
title: "Main function in internal module".into(),
text,
level: diagnostic::Level::Warning,
location: None,
hint: None,
}
}
Warning::InvalidSource { path } => Diagnostic {
title: "Invalid module name".into(),
text: "\
Expand Down Expand Up @@ -1577,3 +1610,22 @@ fn pluralise(string: String, quantity: i64) -> String {
format!("{string}s")
}
}

#[test]
fn deprecated_main_function_pretty() {
let warning = Warning::DeprecatedMainFunction {
module: "apple".into(),
message: "Use some other method please!".into(),
};

insta::assert_snapshot!(insta::internals::AutoName, warning.to_pretty_string());
}

#[test]
fn internal_module_main_function_pretty() {
let warning: Warning = Warning::InternalModuleMainFunction {
module: "internal/picking".into(),
};

insta::assert_snapshot!(insta::internals::AutoName, warning.to_pretty_string());
}