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

AutoFactory generated code is mis-annotating parameters #949

Closed
wesalvaro opened this issue Jan 7, 2021 · 2 comments · Fixed by #950
Closed

AutoFactory generated code is mis-annotating parameters #949

wesalvaro opened this issue Jan 7, 2021 · 2 comments · Fixed by #950
Assignees
Labels
Component: factory P2 type=defect Bug, not working as expected

Comments

@wesalvaro
Copy link

wesalvaro commented Jan 7, 2021

When using the Checker Framework's TYPE_USE Nullable annotation,
AutoFactory is generating code that changes the scoping (?) of the inner-class which causes the annotation to be placed in the wrong location.

import com.my.package.Foo.Bar;
import com.my.package.Foo.Baz;
import org.checkerframework.checker.nullness.qual.Nullable;

@AutoFactory
class MyClass {
  MyClass(
    @Nullable Bar bar,
    @Nullable Baz baz,
    ...
  ) { ... }
}

The above seems to be generating:

import com.my.package.Foo;
import org.checkerframework.checker.nullness.qual.Nullable;

class MyClassFactory {
  MyClassFactory(
    @Nullable Foo.Bar bar,
    @Nullable Foo.Baz baz,
    ...
  ) { ... }
}

Because the build dies with this error:

 scoping construct cannot be annotated with type-use annotation: @org.checkerframework.checker.nullness.qual.Nullable
      @Nullable Foo.Bar bar,
 scoping construct cannot be annotated with type-use annotation: @org.checkerframework.checker.nullness.qual.Nullable
      @Nullable Foo.Baz baz,

Why isn't the generated code using the same scoping setup as the source?

Issue #732 may be related?

@wesalvaro wesalvaro changed the title I have a different (perhaps larger) issue although I'm surprised hasn't come up... AutoFactory generated code is mis-annotating parameters Jan 7, 2021
@eamonnmcmanus eamonnmcmanus self-assigned this Jan 8, 2021
copybara-service bot pushed a commit that referenced this issue Jan 8, 2021
Treating them the same as other annotations leads to problems with nested types like `Map.Entry`. If a field or parameter is a `Map.Entry` that is `@Nullable`, and if `@Nullable` is a `TYPE_USE` annotation, then the declaration must use `Map.@nullable Entry`. JavaPoet will do this correctly if the annotations are applied to the `TypeName` for the declaration.

Also ensure that type annotations are included in the `T` of `Provider<T>` when that appears in field or parameter declarations.

Fixes #949.

RELNOTES=AutoFactory type annotations are now placed correctly even for nested types.
PiperOrigin-RevId: 350804805
copybara-service bot pushed a commit that referenced this issue Jan 8, 2021
Treating them the same as other annotations leads to problems with nested types like `Map.Entry`. If a field or parameter is a `Map.Entry` that is `@Nullable`, and if `@Nullable` is a `TYPE_USE` annotation, then the declaration must use `Map.@nullable Entry`. JavaPoet will do this correctly if the annotations are applied to the `TypeName` for the declaration.

Also ensure that type annotations are included in the `T` of `Provider<T>` when that appears in field or parameter declarations.

Fixes #949.

RELNOTES=AutoFactory type annotations are now placed correctly even for nested types.
PiperOrigin-RevId: 350804805
@netdpb netdpb added Component: factory P2 type=defect Bug, not working as expected labels Jan 8, 2021
@eamonnmcmanus
Copy link
Member

FTR, issue #732 is not related. The problem here is that we're not putting TYPE_USE annotations in the right place when we use JavaPoet. Issue #732 concerns AutoValue, not AutoFactory as here, and AutoValue doesn't use JavaPoet.

I have a fix for this issue in the pipeline.

copybara-service bot pushed a commit that referenced this issue Jan 9, 2021
Treating them the same as other annotations leads to problems with nested types like `Map.Entry`. If a field or parameter is a `Map.Entry` that is `@Nullable`, and if `@Nullable` is a `TYPE_USE` annotation, then the declaration must use `Map.@nullable Entry`. JavaPoet will do this correctly if the annotations are applied to the `TypeName` for the declaration.

Also ensure that type annotations are included in the `T` of `Provider<T>` when that appears in field or parameter declarations.

Fixes #949.

RELNOTES=AutoFactory type annotations are now placed correctly even for nested types.
PiperOrigin-RevId: 350804805
@wesalvaro
Copy link
Author

Thank you for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: factory P2 type=defect Bug, not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants