diff --git a/CHANGELOG.md b/CHANGELOG.md index 638b38b925b..db87404c57e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/compiler-cli/src/build.rs b/compiler-cli/src/build.rs index 6c72ced812c..5650a91e74e 100644 --- a/compiler-cli/src/build.rs +++ b/compiler-cli/src/build.rs @@ -5,7 +5,7 @@ use gleam_core::{ build::{Built, Codegen, NullTelemetry, Options, ProjectCompiler, Telemetry}, manifest::Manifest, paths::ProjectPaths, - warning::WarningEmitterIO, + warning::WarningEmitter, }; use crate::{ @@ -28,14 +28,16 @@ pub fn download_dependencies(paths: &ProjectPaths, telemetry: impl Telemetry) -> } pub fn main(paths: &ProjectPaths, options: Options, manifest: Manifest) -> Result { - 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, + warnings: Rc, ) -> Result { let perform_codegen = options.codegen; let root_config = crate::config::root_config(paths)?; @@ -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, diff --git a/compiler-cli/src/fix.rs b/compiler-cli/src/fix.rs index 51cc5a3e266..acc3f392e3f 100644 --- a/compiler-cli/src/fix.rs +++ b/compiler-cli/src/fix.rs @@ -7,7 +7,7 @@ use gleam_core::{ error::{FileIoAction, FileKind}, paths::ProjectPaths, type_, - warning::VectorWarningEmitterIO, + warning::{VectorWarningEmitterIO, WarningEmitter}, }; use hexpm::version::Version; @@ -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(); diff --git a/compiler-cli/src/run.rs b/compiler-cli/src/run.rs index 46f7e3ff1ca..60a00bd216e 100644 --- a/compiler-cli/src/run.rs +++ b/compiler-cli/src/run.rs @@ -1,8 +1,9 @@ -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}, @@ -10,9 +11,13 @@ use gleam_core::{ 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 { @@ -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) => { @@ -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); @@ -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 { + 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. diff --git a/compiler-core/src/build/project_compiler.rs b/compiler-core/src/build/project_compiler.rs index c4c0d90e26f..f5da0105f12 100644 --- a/compiler-core/src/build/project_compiler.rs +++ b/compiler-core/src/build/project_compiler.rs @@ -109,7 +109,7 @@ pub struct ProjectCompiler { /// The set of modules that have had partial compilation done since the last /// successful compilation. incomplete_modules: HashSet, - warnings: WarningEmitter, + warnings: Rc, telemetry: &'static dyn Telemetry, options: Options, paths: ProjectPaths, @@ -135,6 +135,26 @@ where warning_emitter: Rc, 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, + telemetry: &'static dyn Telemetry, + warning_emitter: Rc, + paths: ProjectPaths, + io: IO, ) -> Self { let packages = packages .into_iter() @@ -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, diff --git a/compiler-core/src/snapshots/gleam_core__warning__deprecated_main_function_pretty.snap b/compiler-core/src/snapshots/gleam_core__warning__deprecated_main_function_pretty.snap new file mode 100644 index 00000000000..50a89b94cde --- /dev/null +++ b/compiler-core/src/snapshots/gleam_core__warning__deprecated_main_function_pretty.snap @@ -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! diff --git a/compiler-core/src/snapshots/gleam_core__warning__internal_module_main_function_pretty.snap b/compiler-core/src/snapshots/gleam_core__warning__internal_module_main_function_pretty.snap new file mode 100644 index 00000000000..0f638a4abd1 --- /dev/null +++ b/compiler-core/src/snapshots/gleam_core__warning__internal_module_main_function_pretty.snap @@ -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. diff --git a/compiler-core/src/type_.rs b/compiler-core/src/type_.rs index 82c123d4cdf..fbb00161573 100644 --- a/compiler-core/src/type_.rs +++ b/compiler-core/src/type_.rs @@ -983,6 +983,7 @@ impl ModuleValueConstructor { #[derive(Debug, Clone)] pub struct ModuleFunction { pub package: EcoString, + pub deprecation: Deprecation, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -1134,6 +1135,7 @@ impl ModuleInterface { Ok(ModuleFunction { package: self.package.clone(), + deprecation: value.deprecation.clone(), }) } diff --git a/compiler-core/src/warning.rs b/compiler-core/src/warning.rs index e133cd1b73c..2688a3261a3 100644 --- a/compiler-core/src/warning.rs +++ b/compiler-core/src/warning.rs @@ -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)] @@ -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: "\ @@ -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()); +}