You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "shubhamvishu (via GitHub)" <gi...@apache.org> on 2024/04/30 09:36:38 UTC
[PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
shubhamvishu opened a new pull request, #13329:
URL: https://github.com/apache/lucene/pull/13329
### Description
In #13328 we got to know `renderSiteJavadoc` task fails for javadocs checks are not working as expected with records ctor and methods(synthetic ones). As pointed by @uschindler its hard to find which ones are synthetic and fix it. Meanwhile, we could temporarily disable the javadoc checks for record ctors and methods till the main issue is resolved.
<!--
If this is your first contribution to Lucene, please make sure you have reviewed the contribution guide.
https://github.com/apache/lucene/blob/main/CONTRIBUTING.md
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
Posted by "shubhamvishu (via GitHub)" <gi...@apache.org>.
shubhamvishu commented on PR #13329:
URL: https://github.com/apache/lucene/pull/13329#issuecomment-2085404981
Nice....this is great!
I think this takes care of record ctors along with synthetic methods due to `ExecutableElement.getEnclosingElement().getEnclosedElements()` returning ctor also as enclosed element in `Elements#recordComponentFor`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13329:
URL: https://github.com/apache/lucene/pull/13329#issuecomment-2085565818
I am closing this in favour of #13332.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13329:
URL: https://github.com/apache/lucene/pull/13329#issuecomment-2084935782
I am working on a solution, but also the enum ctors are not even marked as "mandated" elements.
My idea was to replace the whole method (also for the enums by this:
```java
private boolean isSyntheticEnumMethod(Element element) {
if (elementUtils.getOrigin(element) != Origin.EXPLICIT) {
System.err.println(element.getSimpleName().toString());
return true;
}
return false;
}
```
But the `println` only lists default constructors, which is bad. So I have no idea how to detect those methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13329:
URL: https://github.com/apache/lucene/pull/13329#issuecomment-2084983288
With the above code it was able to detect the default constructor of records, but it did not detect the record component accessor methods (same issue like with enums).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler closed pull request #13329: Temporarily exclude record ctors and methods from javadoc checks
URL: https://github.com/apache/lucene/pull/13329
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13329:
URL: https://github.com/apache/lucene/pull/13329#issuecomment-2085565104
I created a new PR #13332 implementing the linter correctly:
- exclude default ctors
- exclude record accessor methods
- check in level=METHOD that all record components have a documentation (using `@param`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13329:
URL: https://github.com/apache/lucene/pull/13329#issuecomment-2085324323
I have a working solution already which works with records. I just want to figure out how to ideally check for missing `@param` on record components.
I will commit to this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13329:
URL: https://github.com/apache/lucene/pull/13329#issuecomment-2085449503
> Nice! This is neat. Thank you Uwe!
>
> I think this takes care of record ctors along with synthetic methods due to `ExecutableElement.getEnclosingElement().getEnclosedElements()` returning ctor also as enclosed element in `Elements#recordComponentFor`?
No, the ctors are excluded because of the first if statement. Actually regarding the comment "UweSays: It should not happen but it happens!" the following if statements should be all not needed (because the elements are not explicit - but mandated - it still reports them.
In short:
- `Elements#recordComponentFor` tests every method if it is the accessor of a record component, if it returns something != null, we can exclude it
- `Elements#getOrigin(element) != Origin.EXPLICIT` finds all ctors which are not declared
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13329:
URL: https://github.com/apache/lucene/pull/13329#issuecomment-2085327836
Here is my code that works:
```java
/**
* Return true if the method is synthetic enum (values/valueOf) or record accessor method.
* According to the doctree documentation, the "included" set never includes synthetic/mandated elements.
* UweSays: It should not happen but it happens!
*/
private boolean isSyntheticMethod(Element element) {
// exclude all not explicitely declared methods
if (elementUtils.getOrigin(element) != Origin.EXPLICIT) {
return true;
}
// exclude record accessors
if (element instanceof ExecutableElement ex && elementUtils.recordComponentFor(ex) != null) {
return true;
}
// exclude special enum methods
String simpleName = element.getSimpleName().toString();
if (simpleName.equals("values") || simpleName.equals("valueOf")) {
if (element.getEnclosingElement().getKind() == ElementKind.ENUM) {
return true;
}
}
return false;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Temporarily exclude record ctors and methods from javadoc checks [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13329:
URL: https://github.com/apache/lucene/pull/13329#issuecomment-2084939587
Maybe it works for records, because they are "newer". I will add a fake record for testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org