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 support for raw (opaque String) parameter list #760

Closed
vlsi opened this issue Jan 26, 2020 · 9 comments
Closed

Add support for raw (opaque String) parameter list #760

vlsi opened this issue Jan 26, 2020 · 9 comments

Comments

@vlsi
Copy link

vlsi commented Jan 26, 2020

Hi,

MethodSpec.Builder#addParameter(com.squareup.javapoet.ParameterSpec) is nice when parameter types are known.

However, I have a use case when parameters are not really known.

In other words, I want to create a method, and I want to use a simple string for arguments.

For instance:

MethodSpec.methodBuilder(name)
    .addParameters("int x, int y, List z")
    ....

The use case is code generation in javacc/javacc#140
For historical reasons, JavaCC does not really parse user-provided argument types.

For instance, SQL grammar contains the following:

// Parse sql type name that allows precision and scale specifications.
SqlTypeNameSpec SqlTypeName3(Span s) : <-- HERE. result type and formal parameters are not parsed to JavaCC model :(
{
    final SqlTypeName sqlTypeName;
    int precision = -1;
    int scale = -1;
}
{
    (
        (<DECIMAL> | <DEC> | <NUMERIC>) { sqlTypeName = SqlTypeName.DECIMAL; }
    |
        <ANY> { sqlTypeName = SqlTypeName.ANY; }
    )
    ...

In practice, JavaCC's code generator does not care much of the return type and argument type.

It turns out JavaPoet allows to use raw (opaque) string for return type: ClassName.get("", "List<any type>").
However, there's no way to declare a method without knowing the structure of its parameters :(

@vlsi vlsi changed the title Add support for raw (non-parsed) parameter list Add support for raw (opaque String) parameter list Jan 26, 2020
@vlsi
Copy link
Author

vlsi commented Jan 26, 2020

What do you think of the following?

    public final CodeBlock opaqueParameters;

    public MethodSpec.Builder addOpaqueParameters(CodeBlock block) {
      this.opaqueParameters.add(block);
      return this;
    }

@JakeWharton
Copy link
Member

I'm opposed to such an API because it means we need to support it for return types, field types, and then many other places we support types. Additionally, you still need to process the opaque type to emit imports correctly in which case you could just build a proper TypeName. How do you handle imports with these?

@vlsi
Copy link
Author

vlsi commented Jan 26, 2020

How do you handle imports with these?

Just like with CodeBlock#addCode(String, ...).
The opaque data is either not using imports, or it uses fully qualified references, or it assumes the classes are imported.

Note: JavaCC users declare imports manually.
So I don't really mind editing the output with substring.

In other words, the parameter list comes to me in a source form (== plain Java-like string).
So build a proper TypeName is non-trivial, since I would have to support generics and annotations.

Full type grammar is non-trivial (there are annotations, comments, etc), and I don't really need that for my use case. JavaCC does not need to process/analyze arguments either.
I just want to generate a method with user-provided arguments.

The contents of the method would be a mix of user-provided and machine-generated code. I want machine-generated code to be structured (e.g. to avoid unclosed braces), so I want to use JavaPoet for that.

it means we need to support it for return types, field types, and then many other places we support types

types != parameters

There's a workaround for types: ClassName.get("", "whatever I want") returns TypeName that can be used as an opaque type.

For instance, ClassName.get("", "List<String>") just works for me.

@vlsi
Copy link
Author

vlsi commented Jan 27, 2020

many other places

It looks like the impacted cases are the ones that have lists of items.

Two more places are: superinterfaces, and method exceptions
UPD: the current superinterfaces and method exceptions can consume TypeName, so ClassName.get("", "opaque string") works for it as a workaround.
However, having an official opaque TypeName would be great as well.

Note: for for and catch the problem does not exist because JavaPoet does not enforce the layout. In other words, controlFlow is opaque-first (which might be good or bad, but it is what it is)

@Egorand
Copy link
Collaborator

Egorand commented Jan 27, 2020

You should be able to solve this by post-processing generated code, e.g. generate a placeholder ParameterSpec that would be easy to locate and replace it with the opaque string.

@vlsi
Copy link
Author

vlsi commented Jan 27, 2020

Oh, if you mean that as a workaround, then thanks. It might be helpful indeed.
If you mean that as the only possible solution, then it is sad.

For the time being (e.g. while the issue is under consideration), I've copy-pasted JavaPoet to my source tree and added addOpaqueParameters method.

@vlsi
Copy link
Author

vlsi commented Feb 1, 2020

One more opaque case: TypeSpec body. In other words, if half of the TypeSpec exists as a string, then it should be possible to append it.

@JakeWharton
Copy link
Member

JakeWharton commented Feb 1, 2020 via email

@Egorand
Copy link
Collaborator

Egorand commented Feb 9, 2020

Agree, I wouldn't want this kind of API to be a part of core JavaPoet. Closing.

@Egorand Egorand closed this as completed Feb 9, 2020
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

No branches or pull requests

3 participants