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

[Records] Unhelpful warning "Empty block should be documented" for records without components #3316

Closed
jubax opened this issue Nov 15, 2024 · 6 comments · Fixed by #3342
Closed
Assignees
Milestone

Comments

@jubax
Copy link

jubax commented Nov 15, 2024

When you have

sealed interface Foo {
    record Bar1WithParameter(String parameter) implements Foo {
    }
    record Bar2WithoutParameter() implements Foo {
    }
    record Bar3WithParameter(String parameter) implements Foo {
    }
}

Then you get the "empty block" warning for the seconds record (probably because the record has no parameters):

image

This warning should not be shown.

@srikanth-sankaran srikanth-sankaran self-assigned this Nov 15, 2024
@srikanth-sankaran
Copy link
Contributor

I'll take a look.

I have never programmed with records so a naive question. Does a record with no data make sense ? Are there valid use cases for such ??

@srikanth-sankaran srikanth-sankaran added this to the MilestoneNxt milestone Nov 15, 2024
@jubax
Copy link
Author

jubax commented Nov 15, 2024

I'll take a look.

I have never programmed with records so a naive question. Does a record with no data make sense ? Are there valid use cases for such ??

Yes, it is useful for more functional-style result values. E.g. consider

ParseLogFileResult parseLogFileAndExtractErrorLevelMessages(File file) {}

sealed interface ParseLogFileResult {
  record Success(List<String> messagesWithErrorLevel) implements ParseLogFileResult {}
  record NotApplicable_FileDoesNotExist() {}
  record NotApplicable_InvalidLogFileFormat() {}
  record Failure(Exception exception) {}
}

So we are using records as markers here.

@iloveeclipse
Copy link
Member

Oftopic: why not use enums instead?

@jubax
Copy link
Author

jubax commented Nov 15, 2024

Oftopic: why not use enums instead?

Because enums are static. But in the example above I want to return the actual values. With the structure above I can write

    sealed interface ParseLogFileResult {
      record Success(List<String> messagesWithErrorLevel) implements ParseLogFileResult {}
      record NotApplicable_FileDoesNotExist() implements ParseLogFileResult {}
      record NotApplicable_InvalidLogFileFormat() implements ParseLogFileResult {}
      record Failure(Exception exception) implements ParseLogFileResult {}
    }
    
    ParseLogFileResult parseLogFileAndExtractErrorLevelMessages(File file) {
	return new ParseLogFileResult.Success(List.of("a", "b"));
    }
    
    public void test() {
	ParseLogFileResult result = parseLogFileAndExtractErrorLevelMessages(new File("x.log"));
	switch (result) {
        	case ParseLogFileResult.Success(List<String> messagesWithErrorLevel) -> {
        	    System.out.println(messagesWithErrorLevel);
        	}
        	case ParseLogFileResult.NotApplicable_FileDoesNotExist() -> {}
        	case ParseLogFileResult.NotApplicable_InvalidLogFileFormat() -> {}
        	case ParseLogFileResult.Failure(Exception ex) -> { ex.printStackTrace(); }
	}
    }

@srikanth-sankaran
Copy link
Contributor

Thanks for the example. Thought provoking (at least for me.)

@srikanth-sankaran srikanth-sankaran changed the title [warning] "Empty block should be documented" warning for records without parameter [Records] Unhelpful warning "Empty block should be documented" for records without components Nov 25, 2024
@srikanth-sankaran
Copy link
Contributor

srikanth-sankaran commented Nov 26, 2024

See https://bugs.eclipse.org/bugs/show_bug.cgi?id=576806#c1

@MaisiKoleni writes there: The question is if it should make a difference whether a record has at least one component or not. My intuition says that a record without record components (like shown below) should still be considered empty and require a comment explaining what its purpose is.

In the present ticket, we have examples of legitimate use case - so we will course correct on that account.

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 a pull request may close this issue.

3 participants