diff --git a/crates/qbx_lint/tests/cli.rs b/crates/qbx_lint/tests/cli.rs index 869979b..9af748d 100644 --- a/crates/qbx_lint/tests/cli.rs +++ b/crates/qbx_lint/tests/cli.rs @@ -164,6 +164,46 @@ fn ignored_diagnostics_keep_the_files_in_the_analysis() { } } +#[test] +fn configured_imports_define_globals_by_side_for_matching_resources() { + let fixture = Fixture::new(); + let manifest = "fx_version 'cerulean'\ngame 'gta5'\n"; + fixture.write("resources/lib/fxmanifest.lua", format!("{manifest}files {{ 'shared/**.lua', 'client/*.lua' }}\n")); + fixture.write("resources/lib/shared/deep/api.lua", "SharedApi = {}\n"); + fixture.write("resources/lib/client/api.lua", "function ClientApi() end\n"); + for resource in ["[lib]/shop", "other"] { + let scripts = "client_script 'client.lua'\nserver_script 'server.lua'\n"; + fixture.write(&format!("resources/{resource}/fxmanifest.lua"), format!("{manifest}{scripts}")); + fixture.write(&format!("resources/{resource}/client.lua"), "print(SharedApi, ClientApi)\n"); + fixture.write(&format!("resources/{resource}/server.lua"), "print(SharedApi, ClientApi)\n"); + } + fixture.write( + "qbxlint.toml", + "[[overrides]]\nfiles = ['resources/[[]lib[]]/**']\n\ + [overrides.imports]\nshared = ['@lib/shared/**.lua']\nclient = ['@lib/client/*.lua']\n", + ); + let output = fixture.run(&["--format", "json", "--no-fail", "resources"]); + let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let mut undefined: Vec = Vec::new(); + for file in json["files"].as_array().unwrap() { + for diagnostic in file["diagnostics"].as_array().unwrap().iter().filter(|d| d["code"] == "undefined-global") { + let name = diagnostic["message"].as_str().unwrap().split('\'').nth(1).unwrap(); + undefined.push(format!("{} {name}", file["path"].as_str().unwrap().replace('\\', "/"))); + } + } + undefined.sort(); + assert_eq!( + undefined, + [ + "resources/[lib]/shop/server.lua ClientApi", + "resources/other/client.lua ClientApi", + "resources/other/client.lua SharedApi", + "resources/other/server.lua ClientApi", + "resources/other/server.lua SharedApi", + ] + ); +} + #[test] fn relative_and_absolute_config_paths_apply_identical_exclusions_and_overrides() { let fixture = Fixture::new(); @@ -193,6 +233,60 @@ fn relative_and_absolute_config_paths_apply_identical_exclusions_and_overrides() assert_eq!(fixture.read("skip/broken.lua"), b"local n=1\nprint(n)\n"); } +#[test] +fn configured_imports_respect_excluded_files_and_directories() { + let fixture = Fixture::new(); + fixture.write("resources/lib/fxmanifest.lua", "fx_version 'cerulean'\ngame 'gta5'\nfiles { 'api.lua' }\n"); + fixture.write("resources/lib/api.lua", "ExcludedApi = {}\n"); + fixture.write("resources/shop/fxmanifest.lua", "fx_version 'cerulean'\ngame 'gta5'\nclient_script 'client.lua'\n"); + fixture.write("resources/shop/client.lua", "print(ExcludedApi)\n"); + for excluded in ["resources/lib/api.lua", "resources/lib"] { + for pattern in ["@lib/*.lua", "@lib/api.lua"] { + fixture.write("qbxlint.toml", format!("exclude = ['{excluded}']\n[imports]\nclient = ['{pattern}']\n")); + let output = fixture.run(&["--format", "json", "--no-fail", "resources"]); + assert_success(&output); + let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let reported = json["files"].as_array().unwrap().iter().any(|f| { + f["path"].as_str().unwrap().replace('\\', "/").ends_with("shop/client.lua") + && f["diagnostics"].as_array().unwrap().iter().any(|d| d["code"] == "undefined-global") + }); + assert!(reported, "{pattern} leaked a global excluded by {excluded}: {json}"); + } + } +} + +#[test] +fn configured_ox_lib_imports_supply_extensions_and_cache_checks_by_side() { + let fixture = Fixture::new(); + fixture.write( + "shop/fxmanifest.lua", + "fx_version 'cerulean'\ngame 'gta5'\nclient_script 'client.lua'\nserver_script 'server.lua'\n", + ); + fixture.write("shop/client.lua", "print(table.contains({}, 1), PlayerPedId())\n"); + fixture.write("shop/server.lua", "print(table.contains({}, 1))\n"); + for side in ["shared", "client", "server"] { + fixture.write( + "qbxlint.toml", + format!("[rules]\n'qbox/prefer-cache' = 'warning'\n[imports]\n{side} = ['@ox_lib/init.lua']\n"), + ); + let output = fixture.run(&["--format", "json", "--no-fail", "shop"]); + assert_success(&output); + let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + for file in json["files"].as_array().unwrap() { + let path = file["path"].as_str().unwrap().replace('\\', "/"); + let client = path.ends_with("/client.lua"); + if !client && !path.ends_with("/server.lua") { + continue; + } + let codes: Vec<&str> = + file["diagnostics"].as_array().unwrap().iter().map(|d| d["code"].as_str().unwrap()).collect(); + let available = side == "shared" || side == if client { "client" } else { "server" }; + assert_eq!(codes.contains(&"undefined-field"), !available, "{side}: {file}"); + assert_eq!(codes.contains(&"qbox/prefer-cache"), client && available, "{side}: {file}"); + } + } +} + #[test] fn lua_ls_settings_apply_when_no_qbxlint_toml_exists() { let fixture = Fixture::new(); diff --git a/crates/qbx_lua_analysis/src/checks/fivem.rs b/crates/qbx_lua_analysis/src/checks/fivem.rs index 4722e97..2a3fa02 100644 --- a/crates/qbx_lua_analysis/src/checks/fivem.rs +++ b/crates/qbx_lua_analysis/src/checks/fivem.rs @@ -18,7 +18,7 @@ const MAY_RUN_CALLBACKS: &[&str] = pub(super) fn check(input: &FileInput, sink: &mut Sink) { let uses_ox_lib_cache = - input.resource.is_some_and(|r| r.name != "ox_lib" && r.manifest.imports_path("@ox_lib/init.lua", Side::Client)) + input.resource.is_some_and(|r| r.name != "ox_lib" && r.env.imports_path("@ox_lib/init.lua", Side::Client)) && input.side != Some(Side::Server); let mut checker = FiveM { input, diff --git a/crates/qbx_lua_analysis/src/checks/globals.rs b/crates/qbx_lua_analysis/src/checks/globals.rs index de09a3a..d5ae75c 100644 --- a/crates/qbx_lua_analysis/src/checks/globals.rs +++ b/crates/qbx_lua_analysis/src/checks/globals.rs @@ -190,7 +190,7 @@ impl Fields<'_, '_> { let resource_defines = self.input.resource.is_some_and(|r| { r.env.defines_field(table, &field.text) || (OX_LIB_STD_EXTENSIONS.contains(&(table, field.text.as_str())) - && r.manifest.imports_path("@ox_lib/init.lua", self.input.side.unwrap_or(Side::Shared))) + && r.env.imports_path("@ox_lib/init.lua", self.input.side.unwrap_or(Side::Shared))) }); if !defined_here && !resource_defines { self.sink.report(rules::UNDEFINED_FIELD, field.span, format!("'{table}' has no field '{}'", field.text)); diff --git a/crates/qbx_lua_analysis/src/config.rs b/crates/qbx_lua_analysis/src/config.rs index 079ba81..1ee8b32 100644 --- a/crates/qbx_lua_analysis/src/config.rs +++ b/crates/qbx_lua_analysis/src/config.rs @@ -3,9 +3,11 @@ use std::path::{Path, PathBuf}; use globset::{Glob, GlobSet, GlobSetBuilder}; use ignore::gitignore::{Gitignore, GitignoreBuilder}; +use qbx_fivem_data::Side; use serde::Deserialize; use crate::diagnostic::Severity; +use crate::project::split_import; use crate::{lua_ls_config, rules}; pub const CONFIG_FILE_NAMES: &[&str] = &["qbxlint.toml", ".qbxlint.toml"]; @@ -42,15 +44,35 @@ struct RawConfig { ignore_unused_prefix: Option, rules: BTreeMap, overrides: Vec, + imports: Imports, format: qbx_lua_fmt::FormatOptions, } +/// Files a resource runs without an fxmanifest.lua entry, for example through +/// `load(LoadResourceFile(...))`, as `@resource/path` patterns grouped by the side they run on. +#[derive(Clone, Debug, Default, Deserialize)] +#[serde(deny_unknown_fields, default)] +struct Imports { + shared: Vec, + client: Vec, + server: Vec, +} + +impl Imports { + fn entries(&self) -> impl Iterator { + [(&self.shared, Side::Shared), (&self.client, Side::Client), (&self.server, Side::Server)] + .into_iter() + .flat_map(|(patterns, side)| patterns.iter().map(move |p| (p.as_str(), side))) + } +} + #[derive(Clone, Debug, Default, Deserialize)] #[serde(deny_unknown_fields, default)] struct RawOverride { files: Vec, globals: Vec, rules: BTreeMap, + imports: Imports, } #[derive(Clone, Debug)] @@ -58,6 +80,7 @@ struct Override { files: GlobSet, globals: Vec, rules: BTreeMap, + imports: Imports, } #[derive(Clone, Debug)] @@ -74,6 +97,7 @@ pub struct Config { pub notes: Vec, rules: BTreeMap, overrides: Vec, + imports: Imports, } const DEFAULT_EXCLUDES: &[&str] = &["**/node_modules/**", "**/.git/**", "**/[[]builders[]]/**"]; @@ -178,6 +202,15 @@ impl Config { return Err(format!("unknown rule '{code}'")); } } + let imports = std::iter::once(&raw.imports).chain(raw.overrides.iter().map(|o| &o.imports)); + for (pattern, _) in imports.flat_map(Imports::entries) { + let lua = pattern.ends_with(".lua") || pattern.ends_with('*'); + if split_import(pattern).is_none() || !lua { + return Err(format!( + "import '{pattern}' must name Lua files as '@resource/path', such as '@lib/shared/**.lua'" + )); + } + } let exclude = build_globset(DEFAULT_EXCLUDES.iter().copied().chain(raw.exclude.iter().map(String::as_str)))?; let ignore_diagnostics = build_gitignore(&root, &raw.ignore_diagnostics)?; let overrides = raw @@ -188,6 +221,7 @@ impl Config { files: build_globset(o.files.iter().map(String::as_str))?, globals: o.globals, rules: o.rules, + imports: o.imports, }) }) .collect::, String>>()?; @@ -202,6 +236,7 @@ impl Config { notes: Vec::new(), rules: raw.rules, overrides, + imports: raw.imports, }) } @@ -239,6 +274,15 @@ impl Config { } FileConfig { rules, globals, ignore_unused_prefix: self.ignore_unused_prefix.clone() } } + + /// The configured `imports` of the resource whose manifest is `manifest_path`. The scripts of a + /// resource share their globals, so an override adds its imports to every resource whose + /// manifest its `files` patterns match. + pub fn imports_for(&self, manifest_path: &Path) -> Vec<(&str, Side)> { + let relative = self.relative(manifest_path); + let overrides = self.overrides.iter().filter(|o| o.files.is_match(relative)).map(|o| &o.imports); + std::iter::once(&self.imports).chain(overrides).flat_map(Imports::entries).collect() + } } fn build_globset<'a>(patterns: impl Iterator) -> Result { @@ -339,6 +383,31 @@ mod tests { assert!(!config.is_excluded(Path::new("/repo/vendor/lib.lua"))); } + #[test] + fn imports_apply_to_resources_whose_manifest_an_override_matches() { + let config = Config::parse( + r#" + [imports] + shared = ["@lib/shared/**.lua"] + [[overrides]] + files = ["resources/[[]lib[]]/**"] + imports = { client = ["@lib/client/*.lua"], server = ["@oxmysql/lib/MySQL.lua"] } + "#, + PathBuf::from("/repo"), + ) + .unwrap(); + let everywhere = [("@lib/shared/**.lua", Side::Shared)]; + assert_eq!(config.imports_for(Path::new("/repo/resources/chat/fxmanifest.lua")), everywhere); + assert_eq!( + config.imports_for(Path::new("/repo/resources/[lib]/shop/fxmanifest.lua")), + [everywhere[0], ("@lib/client/*.lua", Side::Client), ("@oxmysql/lib/MySQL.lua", Side::Server)] + ); + for pattern in ["lib/shared/a.lua", "@lib", "@lib/web/app.js"] { + let error = Config::parse(&format!("imports = {{ shared = ['{pattern}'] }}"), PathBuf::new()).unwrap_err(); + assert!(error.contains(pattern), "{error}"); + } + } + #[test] fn rejects_unknown_rules() { assert!(Config::parse("[rules]\n\"nope\" = \"off\"", PathBuf::new()).unwrap_err().contains("unknown rule")); diff --git a/crates/qbx_lua_analysis/src/project.rs b/crates/qbx_lua_analysis/src/project.rs index 6d56e90..57f7841 100644 --- a/crates/qbx_lua_analysis/src/project.rs +++ b/crates/qbx_lua_analysis/src/project.rs @@ -7,8 +7,8 @@ use rustc_hash::{FxHashMap, FxHashSet}; use walkdir::WalkDir; use crate::config::Config; -use crate::glob::manifest_glob_match; -use crate::manifest::{Manifest, ScriptEntry, MANIFEST_FILE_NAMES}; +use crate::glob::{is_glob, manifest_glob_match}; +use crate::manifest::{Manifest, MANIFEST_FILE_NAMES}; use crate::scope::{resolve, Resolution}; use crate::summary::{summarize, FileSummary}; @@ -163,6 +163,8 @@ pub struct ResourceEnv { file_scope: FxHashSet, /// `@resource/file.lua` patterns the scripts load at runtime through `lib.load` or `require`. module_imports: FxHashSet, + /// Manifest and configured imports, with the side they run on. + imports: Vec<(SmolStr, Side)>, pub unresolved_imports: Vec, /// Part of the resource is encrypted or unreadable, so neither what it defines nor what it /// uses is known; rules that need the whole picture stay quiet. @@ -201,6 +203,13 @@ impl ResourceEnv { self.module_imports.contains(pattern) } + /// Whether a manifest or configured import loads `path` on `side`. + pub fn imports_path(&self, path: &str, side: Side) -> bool { + self.imports + .iter() + .any(|(pattern, imported_side)| pattern.eq_ignore_ascii_case(path) && imported_side.is_available_on(side)) + } + pub fn defines(&self, name: &str, side: Option) -> bool { match side { Some(Side::Client) => self.client.contains(name), @@ -221,12 +230,40 @@ impl ResourceEnv { pub fn has_unresolved_import_for(&self, side: Option) -> Option<&UnresolvedImport> { self.unresolved_imports.iter().find(|import| side.is_none_or(|s| import.side.is_available_on(s))) } + + /// Adds what an `@resource/path` import provides on `side`: the globals of the files it names, + /// and the usual globals of well-known imports such as `@ox_lib/init.lua`, which also cover + /// imports whose resource is not installed. Any other import that names no file is recorded as + /// unresolved. + pub fn add_import<'a>(&mut self, pattern: &str, side: Side, files: impl IntoIterator) { + self.imports.push((pattern.into(), side)); + let mut resolved = false; + for summary in files { + self.add_summary(summary, Some(side)); + resolved = true; + } + match known_import(pattern) { + Some(known) => known.globals.iter().for_each(|g| self.add_global(&SmolStr::new(g), Some(side))), + None if !resolved => self.unresolved_imports.push(UnresolvedImport { path: pattern.into(), side }), + None => {} + } + } } -/// Finds sibling resources by name so `@resource/file.lua` imports can be followed. +/// Every Lua import of a resource with the side it runs on: the `@resource/path` entries of its +/// manifest, then the `imports` the configuration adds for files it loads at runtime. +pub fn resource_imports<'a>(manifest: &'a Manifest, manifest_path: &Path, config: &'a Config) -> Vec<(&'a str, Side)> { + let own = manifest.imports().filter(|s| s.is_lua()).map(|s| (s.pattern.as_str(), s.side)); + own.chain(config.imports_for(manifest_path)).collect() +} + +/// Finds sibling resources by name so `@resource/file.lua` imports can be followed, and keeps +/// what it read for them, since many resources often import the same files. #[derive(Default)] pub struct ResourceLocator { roots: FxHashMap>, + lua_files: FxHashMap>, + summaries: FxHashMap>, } impl ResourceLocator { @@ -262,38 +299,37 @@ impl ResourceLocator { }); index.get(&name.to_lowercase()).cloned() } + + /// The summaries of the readable files an `@resource/path` import names, where the path may be + /// a manifest glob, read from the resource of that name next to `from_resource`. + pub fn import_summaries(&mut self, from_resource: &Path, pattern: &str, config: &Config) -> Vec<&FileSummary> { + let Some((resource, file)) = split_import(pattern) else { return Vec::new() }; + let Some(root) = self.locate(from_resource, resource) else { return Vec::new() }; + let paths = if is_glob(file) { + let all = self.lua_files.entry(root.clone()).or_insert_with(|| lua_files_under(&root, config)); + all.iter().filter(|path| manifest_glob_match(file, &relative_slash_path(&root, path))).cloned().collect() + } else { + let path = root.join(file); + if config.is_excluded(&path) { + return Vec::new(); + } + vec![path] + }; + for path in &paths { + self.summaries.entry(path.clone()).or_insert_with(|| { + let source = read_source(path).ok()?; + let chunk = parse(&source); + Some(summarize(&chunk, &resolve(&chunk))) + }); + } + paths.iter().filter_map(|path| self.summaries.get(path)?.as_ref()).collect() + } } pub fn split_import(pattern: &str) -> Option<(&str, &str)> { pattern.strip_prefix('@')?.split_once('/') } -/// Adds the globals an `@resource/file.lua` import provides, reading the real file when the -/// resource can be found on disk and falling back to the built-in table of well-known imports. -pub fn add_import(env: &mut ResourceEnv, entry: &ScriptEntry, resource_root: &Path, locator: &mut ResourceLocator) { - if !entry.pattern.ends_with(".lua") { - return; - } - let side = Some(entry.side); - let on_disk = split_import(&entry.pattern).and_then(|(resource, file)| { - let root = locator.locate(resource_root, resource)?; - read_source(&root.join(file)).ok() - }); - if let Some(source) = on_disk { - let chunk = parse(&source); - let resolution = resolve(&chunk); - env.add_summary(&summarize(&chunk, &resolution), side); - if let Some(known) = known_import(&entry.pattern) { - known.globals.iter().for_each(|g| env.add_global(&SmolStr::new(g), side)); - } - return; - } - match known_import(&entry.pattern) { - Some(known) => known.globals.iter().for_each(|g| env.add_global(&SmolStr::new(g), side)), - None => env.unresolved_imports.push(UnresolvedImport { path: entry.pattern.clone(), side: entry.side }), - } -} - pub struct Resource { pub name: String, pub root: PathBuf, @@ -325,8 +361,8 @@ impl Resource { env.add_summary(&file.summary, side); files.push(file); } - for entry in manifest.imports() { - add_import(&mut env, entry, root, locator); + for (pattern, side) in resource_imports(&manifest, &manifest_path, config) { + env.add_import(pattern, side, locator.import_summaries(root, pattern, config)); } let name = root.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); Some(Self { name, root: root.to_path_buf(), manifest_path, manifest, files, env }) diff --git a/docs/reference.md b/docs/reference.md index 4a53031..0b1643f 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -26,6 +26,9 @@ ignore_unused_prefix = "_" "fivem/citizen-prefix" = "warning" "qbox/prefer-cache" = "info" +[imports] +shared = ["@my_lib/shared/**.lua"] + [[overrides]] files = ["tests/**", "**/*.spec.lua"] globals = ["describe", "it"] @@ -43,9 +46,10 @@ quote_style = "preserve" | `exclude` | Additional glob patterns for paths to skip. Skipped files are not analyzed at all, so other files do not see their globals, exports or events. | | `ignore_diagnostics` | Gitignore-style patterns for paths that are analyzed but never reported. | | `globals` | Names supplied at runtime that the linter cannot discover. | +| `imports` | Files every resource runs without an fxmanifest.lua entry, grouped as `shared`, `client` and `server`. See [Runtime imports](#runtime-imports). | | `ignore_unused_prefix` | Locals and arguments with this prefix are exempt from unused checks; defaults to `_`. | | `rules` | Per-rule levels: `off`, `hint`, `info`, `warning` (or `warn`), and `error`. | -| `overrides` | Per-file globals and rule levels, selected by the `files` patterns. Later matching overrides take precedence for rule levels. | +| `overrides` | Per-file globals and rule levels, selected by the `files` patterns, and imports for the resources whose `fxmanifest.lua` the patterns match. Later matching overrides take precedence for rule levels. | | `format` | Formatting options, shown with their defaults above. | The default exclusions include `node_modules`, `.git`, and `[builders]` directory contents. @@ -137,13 +141,45 @@ paths and sides. The analysis combines: 1. Lua and CfxLua runtime definitions from the [bundled stubs](../crates/qbx_fivem_data/stubs). 2. Bundled FiveM native signatures and client/server metadata, including `N_0x...` names. 3. Globals defined by scripts available on the file's side. -4. Globals from manifest imports such as `@resource/file.lua`. +4. Globals from manifest imports such as `@resource/file.lua`, and from configured `imports`. 5. Configured `globals`. An import is read from a sibling resource when available. Otherwise, known imports supply their usual globals, such as `lib` and `cache` for `@ox_lib/init.lua`, `MySQL` for `@oxmysql/lib/MySQL.lua`, and `qbx` for `@qbx_core/modules/lib.lua`. +### Runtime imports + +Some resources run code from another resource without an fxmanifest.lua entry, for example a +loader that calls `load(LoadResourceFile(...))` for each file of a shared library. `imports` lists +those files as `@resource/path` patterns, grouped by the side they run on, and each resource then +sees their globals as if its manifest imported them: + +```toml +[imports] +shared = ["@my_lib/shared/**.lua"] +client = ["@my_lib/client/**.lua"] +server = ["@my_lib/server/**.lua", "@oxmysql/lib/MySQL.lua"] +``` + +Paths may use manifest globs, where `*` stays within a folder and `**` crosses folders. Top-level +`imports` apply to every resource. To limit them to some resources, put them in an override: its +imports apply to each resource whose `fxmanifest.lua` its `files` patterns match, since all +scripts of a resource share their globals. Keeping those resources in one category folder makes +the pattern short: + +```toml +[[overrides]] +files = ["resources/[[]my_lib[]]/**"] + +[overrides.imports] +shared = ["@my_lib/shared/**.lua"] +``` + +In `files` patterns, `[[]` and `[]]` match literal brackets; `[my_lib]` alone would be a character +class. Server scripts do not see the globals of `client` imports, and client scripts do not see +those of `server` imports. Excluded files add nothing. + Files not listed as manifest scripts, including modules loaded through `require` or `lib.load`, use globals from both sides. Files outside a resource are checked without a manifest environment. diff --git a/examples/qbxlint.toml b/examples/qbxlint.toml index c64dc45..4e0c767 100644 --- a/examples/qbxlint.toml +++ b/examples/qbxlint.toml @@ -12,6 +12,13 @@ globals = [] # Locals and arguments starting with this prefix are never reported as unused. ignore_unused_prefix = "_" +# Files resources run without an fxmanifest.lua entry, for example through load(LoadResourceFile(...)). +# Put them in an [[overrides]] entry instead to limit them to some resources. +[imports] +shared = [] +client = [] +server = [] + [rules] # Levels: "off", "hint", "info", "warning", "error". Run `qbx-lint --list-rules` for all codes. "unused-argument" = "off"