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

Add missing spaces in concatenated strings #12967

Merged
merged 1 commit into from Dec 24, 2023
Merged

Add missing spaces in concatenated strings #12967

merged 1 commit into from Dec 24, 2023

Conversation

sabi0
Copy link
Contributor

@sabi0 sabi0 commented Dec 23, 2023

Mostly changing code like this
"expected: " + currentFullFlushDelQueue + "but was: "
to
"expected: " + currentFullFlushDelQueue + " but was: "

But also fixing bugs like "" + i + numDocs => "" + (i + numDocs).

I took the liberty to also do some minor housekeeping in the affected files:

  • corrected grammar and spelling mistakes
  • removed unnecessary type parameters
  • optimized API usage (e.g. "UTF-8" => UTF_8, assertThat() => assertEquals(), etc.)

Copy link
Member

@mikemccand mikemccand left a comment

Choose a reason for hiding this comment

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

Wow, thank you for the attention to detail @sabi0!

I'm curious: how did you uncover all of these corrections? Is there some static analysis tool that you started from?

@@ -236,7 +236,7 @@ synchronized void abort() throws IOException {
}
}

final boolean flushOneDWPT() throws IOException {
boolean flushOneDWPT() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

Ahh because the class itself is already final, great!


public DocIdSetIterator iterator(IndexReader r) throws IOException;
Copy link
Member

Choose a reason for hiding this comment

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

There is another PR about removing the not-needed public in interfaces I think.

@mikemccand mikemccand merged commit 02722ee into apache:main Dec 24, 2023
4 checks passed
@mikemccand
Copy link
Member

I'll backport for 9.10 soon.

@mikemccand mikemccand added this to the 9.10.0 milestone Dec 24, 2023
@sabi0
Copy link
Contributor Author

sabi0 commented Dec 24, 2023

Thank you for checking this so quickly!

I started with a "Whitespace may be missing in string concatenation" inspection in IntelliJ IDEA.
After applying those fixes I went through the changed files again fixing file-level problems reported by the default inspections profile.

@sabi0 sabi0 deleted the missing_spaces branch December 24, 2023 10:24
@mikemccand
Copy link
Member

I started with a "Whitespace may be missing in string concatenation" inspection in IntelliJ IDEA. After applying those fixes I went through the changed files again fixing file-level problems reported by the default inspections profile.

Cool! Thanks for your attention to detail. I wish we could somehow fold these checks into Lucene's static code analysis tooling...

@sabi0
Copy link
Contributor Author

sabi0 commented Dec 28, 2023

https://www.jetbrains.com/qodana/

Open source projects can get a free license for any JetBrains products:
https://www.jetbrains.com/community/opensource/#support

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.

None yet

2 participants