Allow the library builder to work with non-dart (json, xml, yaml, etc...) based on allow build extension

[pratikpparikh]

The current source generator only works with dart code even for LibraryBuilder, which kind not does not make sense. This change will support to generate code for non-dart extension.

[google-cla]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[pratikpparikh]

@googlebot I signed it!

[jakemac53]

This library is really not intended for use on non-Dart files - see the core Builder class from the build system which is probably what you want.

I think we likely want to make the library argument non-nullable here as well when we migrate for null safety, which would be incompatible with this.

[pratikpparikh]

@jakemac53 can you please explain the correct use case for LibraryBuilder as it stands today? It seems to be useless, not a single example of LibraryBuilder. Why limit to Dart files? It be nice if you can explain the need for limitation of such nicely defined abstraction?

[jakemac53]

LibraryBuilder, PartBuilder, and SharedPartBuilder all were built specifically for the case of consuming some Dart code and outputting more Dart code. This is baked into the apis at several levels.

For instance, LibraryBuilder only supports specifying the output extensions, not the input extensions. These are hard coded to be .dart. All of these classes also take Generators as arguments, which are the things that actually do the codegen. These all are given a LibraryReader as the first argument, something which only makes sense if the input extension is a Dart file.

The majority of the functionality the package provides is based around consuming dart code - it doesn't really do anything for you as far as emitting it (other than the shared part stuff).

It might be helpful to understand more what it is you want to use this package for, as opposed to the vanilla build package and its apis? What is this package providing that you see as missing from package:build?

[pratikpparikh]

@jakemac53 thank you for your reply. For my use case I have some generator that use PartBuilder and SharedPartBuilder which take benefit of annotation processing. Using the same codebase I want create a version resilient code generator that will read json specification versions as input and output dart code library for the set of specification and its version.

i.e. we have a domain called ITSM and it has v1, v2, and v3. The generator will generate a dart library that will be version resilient. So if i use the the builder then i can't elegantly reuse the code write for other generator.

[jakemac53]

So if i use the the builder then i can't elegantly reuse the code write for other generator.

You should be able to re-use code here by creating shared functions here? Ultimately they both should just be building up output String objects, there shouldn't be any incompatible concepts between the two?

[pratikpparikh]

@jakemac53, yes should be able to re-use code, but is it only me or does it not make sense to not block non dart input?

[jakemac53]

The question is really "why does it make sense to allow non-Dart input"? I don't see a compelling case for that today, and I do see some arguments against it.

We don't want to broaden the scope of the package unnecessarily, narrowly targeted packages are easier to maintain. We wouldn't want to be hampered from making some design decision we would like to later on because of this choice.

I will give a very concrete example. Soon we will want to migrate this package to null safety. Today, the types on the generate would all be able to stay unchanged - you are always given a non-null library reader and build step. With the change you propose here, the library reader would have to change to be LibraryReader? since it is nullable. That would be breaking for all existing generators, and it would mean in the future all generators have to deal with that nullability, which is not something we want.

So for that reason alone, I would need a very strong counter argument to allow non-dart files here.

[jakemac53]

I am going to go ahead and close this pr - that doesn't mean the discussion has to end but I am pretty convinced that we shouldn't allow this for the reasons listed above.

[pratikpparikh]

@jakemac53 cuz there are real use cases for non-Dart inputs*. You are not broadening the scope of the package, you are making sure that the package is not more widely usable. What design decision are going to be hampered? if the package handles it correctly then nullsafety can be achieved correctly. I agree LibraryReader could be null and the package could easily handle that appropriately in _generateForLibrary. Look it seems you have made up your mind and you want to limit the scope and you have the right to do that. But there are very real use cases that would require non-dart inputs. You can search the gitter and you will find people who have mentioned them, in any case I am not going to discuss this. I will maintain my fork until I find a better abstraction that handles more comprehensive use cases and move on.

[natebosch]

But there are very real use cases that would require non-dart inputs.

Those use cases can use package:build APIs without depending on package:source_gen.

I don't understand how using LibraryBuilder with non-Dart input file is bringing you value. The only thing I can think of is that you get dartfmt run over the output, but you can get that in your own builder with less effort than trying to reuse LibraryBuilder.

The most relevant use case I can see for using source_gen at all with non-Dart inputs is to write Dart output which is included in the .g.dart file that other SharedPartBuilders would use. You can accomplish that without changes in this package by using the same appliesBuilder configuration as you would if you were using a SharedPartBuilder and ensure your output is going to the right place. For instance if you have two source files lib/foo.dart and lib/foo.json and you want to output some dart code into lib/foo.g.dart based on metadata in lib/foo.json you should be able to have

builders:
  my_builder:
    target: "my_builder"
    import: "package:my_builder/builder.dart"
    builder_factories: ["myBuilder"]
    build_extensions: {".json": [".my_output.g.part"]}
    auto_apply: dependents
    build_to: cache
    applies_builders: ["source_gen|combining_builder"]