Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix KCL source ranges to know which source file they point to #4418

Merged
merged 22 commits into from
Nov 7, 2024

Conversation

jtran
Copy link
Collaborator

@jtran jtran commented Nov 7, 2024

Fixes #4240. Addresses #4080.

When we display errors in a file, we need to know which source file it is pointing to. Previously, we assumed there was only ever one source file. A lot of code assumes that it can convert a Token to an error. So I decided to add the file to Token. But we also need the file specified in SourceRange in the AST Node for runtime errors.

In order to not cause a lot of overhead, the file path is interned (i.e. cached within a single run of the executor) and represented as the NewType ModuleId(u32).

0 always represents the top-level file that's executed, with imported files represented by other IDs. Before we send diagnostics to CodeMirror, we filter out anything that points to a file other than ModuleId(0). This is what fixes the editor to not lock up. See src/lang/errors.ts.

Copy link

qa-wolf bot commented Nov 7, 2024

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

Copy link

vercel bot commented Nov 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Nov 7, 2024 4:35pm

@jtran jtran changed the title Fix KCL error in imported code Fix KCL error in imported code to not lock up the editor Nov 7, 2024
@jtran jtran force-pushed the jtran/multi-file-source-ranges branch from 528469a to f3283d7 Compare November 7, 2024 04:24
@jtran jtran changed the title Fix KCL error in imported code to not lock up the editor Fix KCL source ranges to know which source file they point to Nov 7, 2024
@jtran jtran marked this pull request as ready for review November 7, 2024 04:45
Copy link

codecov bot commented Nov 7, 2024

Codecov Report

Attention: Patch coverage is 97.55849% with 24 lines in your changes missing coverage. Please review.

Please upload report for BASE (achalmers/update-insta-snapshots@1802123). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/wasm-lib/kcl/src/ast/types.rs 94.02% 4 Missing ⚠️
src/wasm-lib/kcl/src/executor.rs 92.85% 4 Missing ⚠️
src/wasm-lib/kcl/src/ast/types/source_range.rs 93.33% 3 Missing ⚠️
src/wasm-lib/kcl/src/walk/ast_node.rs 87.50% 3 Missing ⚠️
src/wasm-lib/kcl-test-server/src/lib.rs 0.00% 2 Missing ⚠️
src/wasm-lib/kcl/src/lint/checks/std_lib_args.rs 33.33% 2 Missing ⚠️
src/wasm-lib/kcl/src/parser/parser_impl.rs 99.25% 2 Missing ⚠️
src/wasm-lib/kcl/src/ast/types/condition.rs 0.00% 1 Missing ⚠️
src/wasm-lib/kcl/src/ast/types/none.rs 80.00% 1 Missing ⚠️
src/wasm-lib/kcl/src/std/kcl_stdlib.rs 0.00% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@                         Coverage Diff                         @@
##             achalmers/update-insta-snapshots    #4418   +/-   ##
===================================================================
  Coverage                                    ?   86.28%           
===================================================================
  Files                                       ?       80           
  Lines                                       ?    28361           
  Branches                                    ?        0           
===================================================================
  Hits                                        ?    24472           
  Misses                                      ?     3889           
  Partials                                    ?        0           
Flag Coverage Δ
wasm-lib 86.28% <97.55%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jtran jtran changed the base branch from main to achalmers/update-insta-snapshots November 7, 2024 15:41
@jtran jtran force-pushed the jtran/multi-file-source-ranges branch from 9e50b06 to dfd27dd Compare November 7, 2024 15:45
Base automatically changed from achalmers/update-insta-snapshots to main November 7, 2024 16:20
@jtran jtran force-pushed the jtran/multi-file-source-ranges branch from dfd27dd to f5fb591 Compare November 7, 2024 16:22
@jtran jtran merged commit 9770523 into main Nov 7, 2024
26 checks passed
@jtran jtran deleted the jtran/multi-file-source-ranges branch November 7, 2024 16:23
@franknoirot franknoirot mentioned this pull request Nov 7, 2024
franknoirot pushed a commit that referenced this pull request Nov 7, 2024
* Add ts_rs feature to work with indexmap

* Add feature for schemars to work with indexmap

* Add module ID to intern module paths

* Update code to use new source range with three fields

* Update generated files

* Update docs

* Fix wasm

* Fix TS code to use new SourceRange

* Fix TS tests to use new SourceRange and moduleId

* Fix formatting

* Fix to filter errors and source ranges to only show the top-level module

* Fix to reuse module IDs

* Fix to disallow empty path for import

* Revert unneeded Self change

* Rename field to be clearer

* Fix parser tests

* Update snapshots

* Change to not serialize module_id of 0

* Update snapshots after adding default module_id

* Move module_id functions to separate module

* Fix tests for console errors

* Proposal: module ID = 0 gets skipped when serializing tokens too (#4422)

Just like in AST nodes.

Also I think "is_top_level" communicates intention better than is_default

---------

Co-authored-by: Adam Chalmers <[email protected]>
franknoirot added a commit that referenced this pull request Nov 12, 2024
* Internal: Pass through `sweep.subType` in `expandSweep`

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* implement a simple startSketchOn / offsetPlane lint rule (#4384)

* Bump kittycad-modeling-cmds from 0.2.71 to 0.2.72 in /src/wasm-lib (#4356)

Bumps [kittycad-modeling-cmds](https://github.com/KittyCAD/modeling-api) from 0.2.71 to 0.2.72.
- [Commits](KittyCAD/modeling-api@kittycad-modeling-cmds-0.2.71...kittycad-modeling-cmds-0.2.72)

---
updated-dependencies:
- dependency-name: kittycad-modeling-cmds
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump google-github-actions/upload-cloud-storage from 2.2.0 to 2.2.1 (#4364)

Bumps [google-github-actions/upload-cloud-storage](https://github.com/google-github-actions/upload-cloud-storage) from 2.2.0 to 2.2.1.
- [Release notes](https://github.com/google-github-actions/upload-cloud-storage/releases)
- [Changelog](https://github.com/google-github-actions/upload-cloud-storage/blob/main/CHANGELOG.md)
- [Commits](google-github-actions/upload-cloud-storage@v2.2.0...v2.2.1)

---
updated-dependencies:
- dependency-name: google-github-actions/upload-cloud-storage
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: 49fl <[email protected]>

* Internal fix: make `expandPath` not assume path has associated sweep (#4386)

* Add a test that shows current error within `expandPath`

* Make `expandPath` not assume there is an associated sweep artifact

* Look at this (photo)Graph *in the voice of Nickelback*

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Bump happy-dom from 14.12.3 to 15.10.1 (#4404)

Bumps [happy-dom](https://github.com/capricorn86/happy-dom) from 14.12.3 to 15.10.1.
- [Release notes](https://github.com/capricorn86/happy-dom/releases)
- [Commits](capricorn86/happy-dom@v14.12.3...v15.10.1)

---
updated-dependencies:
- dependency-name: happy-dom
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump @electron-forge/maker-zip from 7.4.0 to 7.5.0 (#4394)

Bumps [@electron-forge/maker-zip](https://github.com/electron/forge) from 7.4.0 to 7.5.0.
- [Release notes](https://github.com/electron/forge/releases)
- [Changelog](https://github.com/electron/forge/blob/main/CHANGELOG.md)
- [Commits](electron/forge@v7.4.0...v7.5.0)

---
updated-dependencies:
- dependency-name: "@electron-forge/maker-zip"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump uuid from 9.0.1 to 11.0.2 (#4393)

Bumps [uuid](https://github.com/uuidjs/uuid) from 9.0.1 to 11.0.2.
- [Release notes](https://github.com/uuidjs/uuid/releases)
- [Changelog](https://github.com/uuidjs/uuid/blob/main/CHANGELOG.md)
- [Commits](uuidjs/uuid@v9.0.1...v11.0.2)

---
updated-dependencies:
- dependency-name: uuid
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump validator from 0.18.1 to 0.19.0 in /src/wasm-lib (#4396)

Bumps [validator](https://github.com/Keats/validator) from 0.18.1 to 0.19.0.
- [Changelog](https://github.com/Keats/validator/blob/master/CHANGELOG.md)
- [Commits](https://github.com/Keats/validator/commits)

---
updated-dependencies:
- dependency-name: validator
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump insta from 1.41.0 to 1.41.1 in /src/wasm-lib (#4400)

Bumps [insta](https://github.com/mitsuhiko/insta) from 1.41.0 to 1.41.1.
- [Release notes](https://github.com/mitsuhiko/insta/releases)
- [Changelog](https://github.com/mitsuhiko/insta/blob/master/CHANGELOG.md)
- [Commits](mitsuhiko/insta@1.41.0...1.41.1)

---
updated-dependencies:
- dependency-name: insta
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump pyo3 from 0.22.5 to 0.22.6 in /src/wasm-lib (#4398)

Bumps [pyo3](https://github.com/pyo3/pyo3) from 0.22.5 to 0.22.6.
- [Release notes](https://github.com/pyo3/pyo3/releases)
- [Changelog](https://github.com/PyO3/pyo3/blob/v0.22.6/CHANGELOG.md)
- [Commits](PyO3/pyo3@v0.22.5...v0.22.6)

---
updated-dependencies:
- dependency-name: pyo3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump thiserror from 1.0.65 to 2.0.0 in /src/wasm-lib (#4397)

Bumps [thiserror](https://github.com/dtolnay/thiserror) from 1.0.65 to 2.0.0.
- [Release notes](https://github.com/dtolnay/thiserror/releases)
- [Commits](dtolnay/thiserror@1.0.65...2.0.0)

---
updated-dependencies:
- dependency-name: thiserror
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump url from 2.5.2 to 2.5.3 in /src/wasm-lib (#4399)

Bumps [url](https://github.com/servo/rust-url) from 2.5.2 to 2.5.3.
- [Release notes](https://github.com/servo/rust-url/releases)
- [Commits](servo/rust-url@v2.5.2...v2.5.3)

---
updated-dependencies:
- dependency-name: url
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump @types/ws from 8.5.12 to 8.5.13 (#4395)

Bumps [@types/ws](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/ws) from 8.5.12 to 8.5.13.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/ws)

---
updated-dependencies:
- dependency-name: "@types/ws"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* File tree to act more like VS Code's file tree (#4392)

* File tree acts as VS Code's file tree

* Adjust test for new expectations

* Remove screenshot

* Actually remove this screenshot

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Fixing directory/file selection logic to deselect folders properly and always hightlight files.  (#4408)

* File tree acts as VS Code's file tree

* Adjust test for new expectations

* Remove screenshot

* Actually remove this screenshot

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* fix: fixed logic for selection

* fix: always show the current file

---------

Co-authored-by: 49lf <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Bump happy-dom from 15.10.1 to 15.10.2 (#4409)

Bumps [happy-dom](https://github.com/capricorn86/happy-dom) from 15.10.1 to 15.10.2.
- [Release notes](https://github.com/capricorn86/happy-dom/releases)
- [Commits](capricorn86/happy-dom@v15.10.1...v15.10.2)

---
updated-dependencies:
- dependency-name: happy-dom
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Change Dependabot PR frequency to weekly (#4424)

* Update insta snapshots (#4423)

No meaningful changes, they just added a field to the frontmatter

* Fix KCL source ranges to know which source file they point to (#4418)

* Add ts_rs feature to work with indexmap

* Add feature for schemars to work with indexmap

* Add module ID to intern module paths

* Update code to use new source range with three fields

* Update generated files

* Update docs

* Fix wasm

* Fix TS code to use new SourceRange

* Fix TS tests to use new SourceRange and moduleId

* Fix formatting

* Fix to filter errors and source ranges to only show the top-level module

* Fix to reuse module IDs

* Fix to disallow empty path for import

* Revert unneeded Self change

* Rename field to be clearer

* Fix parser tests

* Update snapshots

* Change to not serialize module_id of 0

* Update snapshots after adding default module_id

* Move module_id functions to separate module

* Fix tests for console errors

* Proposal: module ID = 0 gets skipped when serializing tokens too (#4422)

Just like in AST nodes.

Also I think "is_top_level" communicates intention better than is_default

---------

Co-authored-by: Adam Chalmers <[email protected]>

* Bump anyhow from 1.0.92 to 1.0.93 in /src/wasm-lib (#4417)

Bumps [anyhow](https://github.com/dtolnay/anyhow) from 1.0.92 to 1.0.93.
- [Release notes](https://github.com/dtolnay/anyhow/releases)
- [Commits](dtolnay/anyhow@1.0.92...1.0.93)

---
updated-dependencies:
- dependency-name: anyhow
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Cut release v0.26.3 (#4427)

* Cut release v0.26.3

* Support new electron-builder env variable requirements for building in release mode

* Bump image from 0.25.3 to 0.25.5 in /src/wasm-lib (#4416)

Bumps [image](https://github.com/image-rs/image) from 0.25.3 to 0.25.5.
- [Changelog](https://github.com/image-rs/image/blob/main/CHANGES.md)
- [Commits](image-rs/image@v0.25.3...v0.25.5)

---
updated-dependencies:
- dependency-name: image
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Nadro/3799/perf (#4145)

* chore: building out perf testing

* chore: adding my printing code for the different formats of the marks

* feat: adding invocation count table

* fix: markOnce iunstead

* fix: typescript additions

* fix: adding more types

* chore: adding telemetry panel as MVP, gonna remove the pane

* chore: view telemetry from command bar in file route and home route

* fix: deleting unused imports

* fix: deleting some unused files

* fix: auto cleanup

* chore: adding other routes, these will need to be moved...

* chore: moving some printing logic around and unit testing some of it

* fix: moving command init

* fix: removing debugging marks

* fix: adding some comments

* fix: fixed a bug with generating the go to page commands

* chore: adding will pages load within the router config

* chore: implementing marks for routes

* fix: auto fixes and checkers

* chore: implemented a route watcher at the root level...

* fix: auto fixes, removing unused code

* chore: timing for syntax highlighting and auto fixes

* fix: didAuth issue and syntax highlighting in the packaged application. Constructor name gets renamed

* fix: fixing typescript checks

* chore: adding mag bar chart icon for telemetry

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* chore: swapped telemetry icon for stopwatch

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* chore: writing telemetry to disk

* fix: auto fixers

* chore: getting args parsed for cli flags and writing telemetry file

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* chore: swapped mark for markOnce since we infinitely write marks to a JS array... need to solve this run time marking in another way. We only need this for startup right now

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* chore: writing raw marks to disk as well

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* fix: cleaned up the testing names

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* Fix fmt and codespell

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* fix: moving this route loader data stuff

* chore: adding comment

* fix: fmt

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* empty :(

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* empty :(

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: 49fl <[email protected]>

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: 49fl <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Paul Tagliamonte <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Kevin Nadro <[email protected]>
Co-authored-by: Jonathan Tran <[email protected]>
Co-authored-by: Adam Chalmers <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KCL errors in imported files lock up the editor
2 participants