feat(diagnostic): report unresolved require modules

[lewis6991]

Add an unresolved-require diagnostic for require("...") calls that
resolve to a string module path but cannot be found in the module index.

Extend the existing require visibility checker to emit
unresolved-require when module lookup fails.

[gemini-code-assist]

Summary of Changes

Hello @lewis6991, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new diagnostic feature that helps developers identify require calls referencing modules that do not exist within the project's module index. By extending an existing checker and adding a new diagnostic code, the system can now proactively warn about potential runtime errors caused by unresolved module dependencies, improving code reliability and maintainability.

Highlights

• New Diagnostic Code: Introduced a new diagnostic code, UnresolvedRequire, to identify require calls for modules that cannot be found.
• Enhanced Module Visibility Checker: Enhanced the RequireModuleVisibilityChecker to detect and report UnresolvedRequire diagnostics when a module path cannot be resolved.
• Default Severity: Established a default severity of WARNING for the new UnresolvedRequire diagnostic.
• Unit Tests: Added comprehensive unit tests to validate the functionality of the UnresolvedRequire diagnostic, covering resolved, unresolved, and non-literal require scenarios.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• crates/emmylua_code_analysis/src/diagnostic/checker/require_module_visibility.rs
  • Added DiagnosticCode::UnresolvedRequire to the list of codes handled by RequireModuleVisibilityChecker.
  • Modified check_require_call_expr to emit an UnresolvedRequire diagnostic when semantic_model.get_db().get_module_index().find_module(&module_path) returns None.
• crates/emmylua_code_analysis/src/diagnostic/lua_diagnostic_code.rs
  • Defined a new DiagnosticCode::UnresolvedRequire enum variant.
  • Set the default severity for DiagnosticCode::UnresolvedRequire to DiagnosticSeverity::WARNING.
• crates/emmylua_code_analysis/src/diagnostic/test/mod.rs
  • Included the new unresolved_require_test module.
• crates/emmylua_code_analysis/src/diagnostic/test/unresolved_require_test.rs
  • Added a new test file containing test_unresolved_require, test_resolved_require, and test_non_literal_require to verify the new diagnostic.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a new unresolved-require diagnostic to report require calls for modules that cannot be found. The implementation extends the existing RequireModuleVisibilityChecker to handle this new check. When a module path from a require call with a string literal argument cannot be resolved, a WARNING is now correctly emitted. The changes are well-implemented and include tests covering cases for resolved, unresolved, and non-literal module paths.

[xuhuanzy]

This check is unnecessary; we can import .dll files or packages located outside the project path.
If it must be included, it should be disabled by default.

[lewis6991]

This check is unnecessary; we can import .dll files or packages located outside the project path.

But then these are not type checked and you get unknown everywhere? If you are loading object files, then surely you must also have accompanying meta files to go with them?

[xuhuanzy]

This is only an ideal scenario; in the current ecosystem, there exists a vast number of third-party libraries with undeclared types.

[lewis6991]

What benefit do you get from this tool if libraries can't be resolved? So many things will not work: hover, goto, most diagnostics, etc. Having a single warning for each unresolved module shouldn't be that bad. It's certainly not unnecessary. And if it is that bad, what is so bad about downgrading the warning per project? I find it's much better for checks to be strict by default, rather than requiring users to enable everything. Selectively downgrading is much safer.

I understand that some environments do not have very good typing for the libraries they import, and that is why I made this a warning instead of an error.

[xuhuanzy]

If the library path is incorrect, lua itself will report an error during execution, meaning we assume each module actually exists.

[lewis6991]

Sorry I don't understand this point.

One of the main uses of emmylua (and all static analysis tools) is to catch issues statically (early) rather than at runtime (late).

[xuhuanzy]

Have you considered the scenario when importing a dll? What about the case where require is overridden?
We cannot assume that this module does not exist. If it were absent, the runtime would definitely throw an error, which in turn implies that the module must exist.

[xuhuanzy]

If you really want to join, then its warning level needs to be Hint.

[CppCXY]

I think that since we're going to diagnose require, ambiguous require calls should also be detected.
Requiring every C dynamic library to have a meta file is difficult, but we can, following the global approach, define a diagnostic exclusion list. With these two configuration options in place, I believe this diagnostic can be a warning — or even an error.

[xuhuanzy]

I think that since we're going to diagnose require, ambiguous require calls should also be detected. Requiring every C dynamic library to have a meta file is difficult, but we can, following the global approach, define a diagnostic exclusion list. With these two configuration options in place, I believe this diagnostic can be a warning — or even an error.

这样做那么你每个小脚本在引用到 luarocks 时都要建立一个 .emmyrc.json, 这很明显是不可能的, 最多 hint 得了, 有强需求才需要在 .emmyrc.json 提升到 error

[lewis6991]

Have you considered the scenario when importing a dll?

Meta files should be provided so the module can be resolved, even if the meta file just returns any. Otherwise, you can use @diagnostic disable. My codebases use @diagnostic disable for lots of cases, especially around dynamic code.

What about the case where require is overridden?

If it's overridden, then surely it must still provide the same interface? Otherwise, the code is too dynamic and isn't very good to use with emmylua.

We cannot assume that this module does not exist. If it were absent, the runtime would definitely throw an error, which in turn implies that the module must exist.

The check doesn't say the module doesn't exist, just that the checker cannot resolve it, which is important and worth having a warning for. It either indicates that the checkers environment isn't setup correctly, or there is a genuine typo in the module name.