Skip to content

Return non-zero exit code when missing SemanticDB files #332

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 6, 2021
Merged

Return non-zero exit code when missing SemanticDB files #332

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/LsifBuildTool.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,18 +160,20 @@ class LsifBuildTool(index: IndexCommand) extends BuildTool("LSIF", index) {
.isDirectory(targetroot.resolve("META-INF"))
if (errors.nonEmpty && !isSemanticdbGenerated) {
CommandResult(1, Nil)
} else if (errors.nonEmpty && isSemanticdbGenerated) {
index
.app
.reporter
.info(
"Some SemanticDB files got generated even if there were compile errors. " +
"In most cases, this means that lsif-java managed to index everything " +
"except the locations that had compile errors and you can ignore the compile errors." +
errors.mkString("\n")
)
} else {
if (errors.nonEmpty && isSemanticdbGenerated) {
index
.app
.reporter
.info(
"Some SemanticDB files got generated even if there were compile errors. " +
"In most cases, this means that lsif-java managed to index everything " +
"except the locations that had compile errors and you can ignore the compile errors." +
errors.mkString("\n")
)
}
CommandResult(0, Nil)
}
CommandResult(0, Nil)
}

Comment on lines +174 to 178
Copy link
Contributor

@Strum355 Strum355 Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im confused about this, does not returning an explicit CommandResult result in the exit code being set to non-zero?

Copy link
Contributor Author

@olafurpg olafurpg Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CommandResult on line 162 was previously discarded because the if/else wasn't the last statement

Copy link
Contributor

@Strum355 Strum355 Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh so thats a thing in scala 🤔 TIL

Copy link
Contributor Author

@olafurpg olafurpg Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are compiler flags to turn that into an error but I don't like using it because there are too many false positives (lots of side-effecting methods return non-Unit values)

Copy link
Contributor Author

@olafurpg olafurpg Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably messed up because I've been writing too much Java/Go lately with early returns. I don't remember being hit by this in the past, it's usually a non-issue in my experience.

Copy link
Contributor

@Strum355 Strum355 Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It mightve been introduced by me here

Copy link
Contributor Author

@olafurpg olafurpg Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, hehe looks like that indeed 😜

private def compileScalaFiles(
Expand Down