Comments (11)
Yes definitely! It wasn't super reproducible but I'll let you know if I see it again.
from error-prone.
Just FYI I still haven't seen this error again, so feel free to close.
from error-prone.
Thanks. Most of the actual Error Prone team is OOO this week, but I wrote this check, so I'm seeing how easily I can have a look.
Given how much Google's security policies discourage the use of Docker, I've been trying to see if there's a shorter path to reproducing this. I've managed to install scala and sbt, kick off sbt
from within server
, and run build
. That built successfully. Do you see the failure you posted at head (e9f8449aaa13f6402ad41fb3acc14a8b33ad8eff in my clone), or does it appear only with local changes?
from error-prone.
In any case, this whole failure is interesting:
- https://github.com/google/error-prone/blob/v2.23.0/core/src/main/java/com/google/errorprone/bugpatterns/nullness/NullArgumentForNonNullParameter.java#L102
- https://github.com/google/error-prone/blob/v2.23.0/core/src/main/java/com/google/errorprone/bugpatterns/nullness/NullnessUtils.java#L263
- https://github.com/google/error-prone/blob/v2.23.0/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java#L1207
It looks to me like we have a symbol for which isConstructor
is true
but for which enclosingClass
returns null
. At least under JDK11 (which might be what sbt
is using on my machine?), I don't see how we could have a MethodSymbol
with a null owner
(which is what enclosingClass
returns) without getting an NPE in the MethodSymbol
constructor... unless somehow it gets overwritten later?
As an aside, I'm not sure that my enclosingClass(constructedClass)
check could possibly work: If I'm reading it right, that should call constructedClass.enclClass()
, which should return constructedClass
itself....
from error-prone.
(isContructor()
is implemented as a check against the name <init>
. I suppose it's possible that something uses that as a name for a non-constructor? Do you have Scala deps? That still seems like a long shot. I'm probably missing something obvious.)
from error-prone.
I realized I misspoke in my original report: The error happens when running browser tests locally, not when running the server. https://github.com/civiform/civiform/wiki/Testing#functional-browser-tests are our browser test directions (specifically bin/run-browser-test-env
).
But, it actually looks like I can no longer reproduce the error. Earlier today, I was working on a few different local PRs (civiform/civiform#6254, then civiform/civiform#6272 which is stacked on top) and I had one of those PR branches checked out when I started seeing the error. What I just did was:
- Checkout main
- run
bin/run-browser-test-env
-- works fine. - Checkout civiform/civiform#6254.
- run
bin/run-browser-test-env
-- works fine. - Checkout civiform/civiform#6272.
- run
bin/run-browser-test-env
-- works fine.
It seems like something got temporarily mangled, but checking out main un-mangled it somehow?
Since I can't repro and don't have good repro steps, feel free to close as not reproducible.
from error-prone.
Thanks. I'll see if the aforementioned real Error Prone team has any thoughts upon their return. It does seem believable that some file got mangled or something.
In any case, I am going to look more into this whole "I'm not sure that my enclosingClass(constructedClass)
check could possibly work" concern. So we will get something out of this one way or another :)
from error-prone.
Oh, I'm wrong: It works fine because the call to enclClass()
is made on sym.owner
, not on sym
. (It's possible that ASTHelpers.enclosingClass()
was implementing the same unfortunate behavior as enclClass()
up until cl/153079032, when cushon@ gave it the behavior that I need.)
I should probably still have a test for this.
And this is a reminder that it's nice not to have to deal with enclosing classes at all.... Really, the more checking that we can do just based on questions like "Is the target type here a primitive type?" (like https://errorprone.info/bugpattern/NullTernary), the better. Probably NullArgumentForNonNullParameter
should handle primitive types differently or leave that to another future checker. Anyway, I digress.
from error-prone.
I don't have any theories about how that NPE could have happened. I don't know much about the interaction between sbt and javac. My current best idea is to add a more descriptive assertion that would catch that NPE, and see if it happens again.
from error-prone.
@caitlinshk I added an assertion that should print more information if this happens again and released that change as v2.24.1
, can you try updating your build to use the latest version and let us know if you see this crash again?
from error-prone.
Thanks for following up!
from error-prone.
Related Issues (20)
- class com.sun.tools.javac.code.Type$ClassType cannot be cast to class com.sun.tools.javac.code.Type$ArrayType HOT 6
- `JUnitIncompatibleType` on 2.27.0 throws `ClassCastException` for `com.sun.tools.javac.code.Type$TypeVar` around `assertArrayEquals` HOT 1
- False alarm from ClassInitializationDeadlock for inner enum implementing outer interface with default method HOT 4
- [Feature Request] Add check warning against mocking record types HOT 1
- ObjectEqualsForPrimitives should not suggest to convert boxed equals on double or float values HOT 3
- Suggest to use `Double.compare` or `Float.compare` when ObjectEqualsForPrimitives is triggered on floating point primitives
- Patching no longer works for experimental checks like StatementSwitchToExpressionSwitch HOT 1
- False UnusedVariable warning for method parameters only used in overridden implementations HOT 3
- FormatStringAnnotation checker is annoyingly stricter than FormatString HOT 2
- Jdk 23 now broken with Error prone after github updates to newer cut yesterday HOT 4
- JDK 23 compatibility HOT 1
- [PatternMatchingInstanceof] False-positives
- Refaster includes fully qualified reference to the `@AfterTemplate` to local variable inside the template HOT 4
- Dspace mvn compilation error HOT 6
- BugPattern: ParameterName HOT 3
- SelfAssignment false positive with casting a float variable to int HOT 1
- An unhandled exception was thrown by the Error Prone static analysis plugin: BugPattern: InconsistentCapitalization HOT 1
- UnnecessaryDefaultInEnumSwitch does not work with enhanced switch statement for cases with multiple values
- False positives by UnusedVariable on unnamed variables (Java 22)
- An unhandled exception was thrown by the Error Prone static analysis plugin. HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from error-prone.