-
Notifications
You must be signed in to change notification settings - Fork 326
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
Space-precedence does not apply to value-level operators #10597
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,6 +75,7 @@ | |
<edition>.yaml: | ||
enso.bundle.template: | ||
launcher-manifest.yaml: | ||
lib: | ||
engine/: | ||
runner/: | ||
src/: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,4 +5,5 @@ use crate::prelude::*; | |
// === Export === | ||
// ============== | ||
|
||
pub mod enso_linter; | ||
pub mod parser; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
use super::*; | ||
|
||
use ide_ci::programs::cargo; | ||
use ide_ci::programs::Cargo; | ||
|
||
use crate::paths::generated::RepoRoot; | ||
|
||
const LINTER_CRATE_NAME: &str = "enso-parser-debug"; | ||
const LINTER_BIN_NAME: &str = "check_syntax"; | ||
const ENSO_EXTENSION: &str = ".enso"; | ||
|
||
pub fn cargo_run_linter_cmd(repo_root: &Path) -> Result<Command> { | ||
let mut ret = Cargo.cmd()?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While having such a linter may satisfy our current development needs, I believe the right linter shall be integrated into the There is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's important for the compiler to be able to emit syntax warnings. I think it's also useful to have a simple standalone linter--when linting fails in CI, it's nice if it happens right away, not after building the engine. It isn't a new CI task; it's part of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Indeed. However such a linter has to be available to Enso users, not just to us. E.g. it cannot be part of How will we expose the linter to Enso end users? What needs to be changed to make that happen? Can you report an issue describing what needs to be done? Can you implement it (and remove current There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The standalone linter is only for us, to support our CI process. It can provide results before the engine is finished building. For that purpose, it cannot be part of |
||
ret.current_dir(repo_root) | ||
.apply(&cargo::Command::Run) | ||
.apply(&cargo::Options::Package(LINTER_CRATE_NAME.into())) | ||
.apply(&cargo::RunOption::Bin(LINTER_BIN_NAME.into())) | ||
.apply(&cargo::RunOption::Release); | ||
Ok(ret) | ||
} | ||
|
||
fn collect_source_paths(repo_root: &RepoRoot) -> Result<Vec<PathBuf>> { | ||
let glob_pattern = format!("**/*{ENSO_EXTENSION}"); | ||
let source_roots = [&repo_root.distribution.lib.path, &repo_root.test.path]; | ||
let glob_paths: Vec<_> = source_roots | ||
.into_iter() | ||
.map(|path| glob::glob(path.join(&glob_pattern).as_str())) | ||
.collect::<std::result::Result<_, _>>()?; | ||
Ok(glob_paths.into_iter().flatten().collect::<std::result::Result<_, _>>()?) | ||
} | ||
|
||
pub async fn lint_all(repo_root: RepoRoot) -> Result<()> { | ||
let sources = collect_source_paths(&repo_root)?; | ||
cargo_run_linter_cmd(&repo_root)?.arg("--").args(sources).run_ok().await?; | ||
Ok(()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I read this correctly that
warnings
are propagated to TypeScript. How are the warnings propagated to CLI? I don't see any changes inTree
. How will the CLI report these warnings?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is
Tree.getWarnings
, however they are serialized as warning IDs (plus warning-specific details), with a separate table providing the string/template to display each warning ID. I have not yet implemented the JNI to turn this in to agetMessage
for theWarning
object.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to expose the linter via
enso-engine*/bin/enso
we will need a way to access the warnings from JVM.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, expect a PR soon. I just wanted to get this part merged since it touches a lot of files.