Add create unknown module code action #4888

tarkah · 2025-08-28T06:23:54Z

Adds a new code action to create an unknown module. This allows you to easily define new modules without having to leave your editor.

Screen.Recording.2025-08-27.at.11.19.35.PM.mov

GearsDatapacks · 2025-08-28T17:58:34Z

Hi, thank you for this! This seems like something we want, but we generally don't accept unplanned PRs. Would you mind making an issue about this, so we can discuss how exactly we want this to work? I feel I would want this code action available for all modules, not just sub-modules of the current one, though others may have different opinions.

tarkah · 2025-08-28T19:16:45Z

Absolutely! I've moved this to draft for now and created an issue here: #4894

lpil · 2025-08-29T10:57:19Z

Is this ready for review?

tarkah · 2025-08-29T13:52:27Z

Is this ready for review?

Yup! I've just moved it off draft.

lpil

Thank you! This is looking really nice. I've left a small comments inline, and the build is failing currently.

Would you mind updating the changelog file too please 🙏

lpil · 2025-08-31T09:05:40Z

compiler-core/src/language_server/code_action.rs

+/// ```gleam
+/// // foo.gleam
+/// // Diagnostic: Unknown module
+/// import foo/bar/baz


nit: Remove foobar references please, it has a not-very-cheery WW2 association so we avoid it in favour of more lighthearted examples.

lpil · 2025-08-31T09:06:23Z

compiler-core/src/language_server/code_action.rs

+///
+/// ```gleam
+/// // foo/bar/baz.gleam
+/// ```


These two code examples are not very clear, so let's describe this code action using words instead.

lpil · 2025-08-31T09:13:14Z

compiler-core/src/language_server/code_action.rs

+            .input_path
+            .as_str()
+            .strip_suffix(&format!("{}.gleam", self.module.name))
+            .expect("origin is ancestor of module path");


Path manipulation can only happen in the ProjectPaths class, so pass that into the code action class from the LS engine instance and use module.origin to determine which directory to use.

This also enables us to avoid panicking, which we always try to avoid.

lpil · 2025-08-31T09:14:56Z

compiler-core/src/language_server/code_action.rs

+            let uri = Url::from_file_path(format!("{origin_path}/{}.gleam", unknown_module.name))
+                .expect("origin path is absolute");
+
+            CodeActionBuilder::new(format!("Create module {}.gleam", unknown_module.name))


This should either be Create module wibble/wobble or Create src/wibble/wobble.gleam as module names don't have a .gleam suffix, Gleam file paths do.

lpil · 2025-08-31T09:16:02Z

compiler-core/src/language_server/code_action.rs

+        });
+
+        for unknown_module in unknown_modules {
+            if !self.code_action_span.intersects(*unknown_module.location) {


Contains rather than intersects please. We don't want to trigger this if someone selects a larger block that happens to include this error.

compiler-core/src/language_server/code_action.rs

tarkah · 2025-09-03T03:49:30Z

Thanks for the feedback @lpil! I've addressed your comments, let me know if there is anything else that should be addressed.

lpil

Fantastic! Thank you! Looks really good.

I missed in the last review that there are no tests, sorry about that. Could you add tests, including ones that make sure that src, test, and dev are appropriately picked for the new file.

When you are ready for a review please un-draft this PR and tag me for a review. Thank you!

compiler-core/src/language_server/code_action.rs

tarkah · 2025-09-04T05:54:29Z

@lpil Thanks! Tests are added. Let me know what you think of the refactoring needed to support defining an origin & module with TestProject::at in a backwards compatible way.

lpil

Thank you, very nice. The tests look great, but I've left some notes inline about the changes to the test API. Once that is sorted we can merge, thanks!

lpil · 2025-09-04T12:44:57Z

compiler-core/src/language_server/tests.rs

+    Module(Origin, &'a str, Position),
+}
+
+impl From<Position> for Cursor<'_> {


Never implement traits in this codebase please.

lpil · 2025-09-04T12:46:28Z

compiler-core/src/language_server/tests.rs

+                .expect("Module doesn't exist")
+        };
+
+        let ((mut engine, params), code) = match cursor {


Rather than overloading this function please create a new function for positioning within a different file, if needed.

lpil · 2025-09-04T12:48:06Z

compiler-core/src/language_server/tests/action.rs

+
+#[test]
+fn create_unknown_module_under_src() {
+    assert_code_action_file_operations!(


Could we not teach the existing macro how to understand these file operation changes too? It has the nice output which we want to have here, and it would be good to to have to maintain twice as many macros.

lpil · 2025-09-04T12:51:51Z

compiler-core/src/language_server/tests.rs


-        let response = engine.compile_please();
-        assert!(response.result.is_ok());
+        let _response = engine.compile_please();


🔴 Compile errors no longer cause the tests to fail!

This code action requires the type error to be present, positioned_with_io allows them so I can test when invoking code actions in self.src but if we assert on compiler errors under positioned_with_io_in_src/dev/test then I can't trigger this code action within specific modules.

How do you suggest we go about that?

We have existing code actions that only trigger when code fails to compile, how are those tested?

They all only use TestProject::at / positioned_with_io which doesn't make this assertion

Hey @lpil, friendly bump. How'd you like to move forward on this?

tarkah · 2025-09-05T21:46:47Z

@lpil Appreciate the great feedback. I've addressed your comments, though see #4888 (comment)

tarkah · 2025-10-04T03:43:48Z

Hey @lpil checking in on this, can I get another review / finalize a decision on #4888 (comment)?

I've rebased and this is otherwise good to go.

lpil

Sorry, I don't understand why so many any so large changes have been made to the test APIs, there doesn't seem to be anything particularly unusual about this new code action.

Why is it that the tests need to be made so much more complex?

tarkah · 2025-10-22T16:59:52Z

Why is it that the tests need to be made so much more complex?

All changes revolve around needing a TestProject::in_module_at function that allows me to trigger the code action from any origin & module. TestProject::at only allows executing code actions from the root src module.

This code action's behavior is dependent on which origin it is invoked in and I need to test that the new module is properly created within that origin. It also seems generally useful to be able to trigger a code action from anywhere, not just the root src module, so future tests should be able to benefit from this.

I'm happy to abandon these changes and only test the code action against Origin::Src but then we don't have great test coverage on it.

tarkah marked this pull request as draft August 28, 2025 19:14

tarkah mentioned this pull request Aug 28, 2025

LSP code action to create unknown module #4894

Open

tarkah force-pushed the feat/create-unknown-module-code-action branch from ac76b78 to 78681dc Compare August 29, 2025 01:25

tarkah marked this pull request as ready for review August 29, 2025 13:52

lpil reviewed Aug 31, 2025

View reviewed changes

tarkah force-pushed the feat/create-unknown-module-code-action branch from 78681dc to 7fcc8f7 Compare September 3, 2025 03:45

tarkah requested a review from lpil September 3, 2025 03:52

lpil reviewed Sep 3, 2025

View reviewed changes

compiler-core/src/language_server/code_action.rs Outdated Show resolved Hide resolved

lpil marked this pull request as draft September 3, 2025 11:54

tarkah force-pushed the feat/create-unknown-module-code-action branch from 7fcc8f7 to 332f6ed Compare September 4, 2025 05:51

tarkah marked this pull request as ready for review September 4, 2025 05:51

tarkah requested a review from lpil September 4, 2025 05:51

tarkah force-pushed the feat/create-unknown-module-code-action branch from 332f6ed to f254f5e Compare September 4, 2025 05:55

lpil reviewed Sep 4, 2025

View reviewed changes

tarkah force-pushed the feat/create-unknown-module-code-action branch from f254f5e to 4d373c3 Compare September 5, 2025 21:41

tarkah requested a review from lpil September 5, 2025 21:45

tarkah force-pushed the feat/create-unknown-module-code-action branch 2 times, most recently from 4f7ca62 to 383f986 Compare September 11, 2025 21:32

tarkah force-pushed the feat/create-unknown-module-code-action branch from 383f986 to 655236a Compare October 4, 2025 03:41

lpil reviewed Oct 22, 2025

View reviewed changes

tarkah force-pushed the feat/create-unknown-module-code-action branch from 655236a to 5a5e1cd Compare October 22, 2025 17:22

Add create unknown module code action

cff104a

tarkah force-pushed the feat/create-unknown-module-code-action branch from 5a5e1cd to cff104a Compare October 22, 2025 17:26

Uh oh!

Add create unknown module code action #4888

Are you sure you want to change the base?

Add create unknown module code action #4888

Uh oh!

Conversation

tarkah commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GearsDatapacks commented Aug 28, 2025

Uh oh!

tarkah commented Aug 28, 2025

Uh oh!

lpil commented Aug 29, 2025

Uh oh!

tarkah commented Aug 29, 2025

Uh oh!

lpil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tarkah commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lpil left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tarkah commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lpil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarkah Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarkah commented Sep 5, 2025

Uh oh!

tarkah commented Oct 4, 2025

Uh oh!

lpil left a comment

Choose a reason for hiding this comment

Uh oh!

tarkah commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

tarkah commented Aug 28, 2025 •

edited

Loading

tarkah commented Sep 3, 2025 •

edited

Loading

lpil left a comment •

edited

Loading

tarkah commented Sep 4, 2025 •

edited

Loading

tarkah Sep 7, 2025 •

edited

Loading

tarkah commented Oct 22, 2025 •

edited

Loading