format function on CodeWriter which returns String

[trivikr]

Is your feature request related to a problem? Please describe.

• JavaScript SDK team would like to reduce bundle size of JavaScript SDK v3 by removing unnecessary variables
• One such way is to remove some local redundant variables like wrappedItem from ser-deser functions (code)
  • First part of this is using reusable function like getArrayIfSingleItem (commit)
  • The second part of it is to pass output pass call value returned by getArrayIfSingleItem to another function (experimental commit)

Describe the solution you'd like

• CodeWriter exposes a function (say format) which returns formatted string.
• This String can later be passed to

Existing write function: https://github.com/awslabs/smithy/blob/34d51e3f7f9c351ed0867a816e082ba3ac56cc4c/smithy-utils/src/main/java/software/amazon/smithy/utils/CodeWriter.java#L925-L945

This function can be as follows:

    public final CodeWriter write(Object content, Object... args) {
        String value = format(content, args);
        String[] lines = value.split(newlineRegexQuoted, -1);

        // Indent lines and strip excessive newlines.
        for (String line : lines) {
            currentState.writeLine(line + newline);
        }

        return this;
    }

    public final String format(Object content, Object... args) {
        return formatter.format(content, currentState.indentText, this, args);
    }

Describe alternatives you've considered

• Using String.format instead which might surface bugs in future
• Get around by writing complex CodeGen code using just the write method

[trivikr]

Current example function in JavaScript SDK v3:

const deserializeAws_restXmlNestedStringList = (
  output: any,
  context: __SerdeContext
): string[][] => {
  const contents: any = [];
  (output || []).map((entry: any) => {
    const wrappedItem =
      entry["member"] instanceof Array ? entry["member"] : [entry["member"]];
    contents.push(deserializeAws_restXmlStringList(wrappedItem, context));
  });
  return contents;
};

The format function would be helpful to simplify it by removing variables contents and wrappedItem:

const deserializeAws_restXmlNestedStringList = (
  output: any,
  context: __SerdeContext
): string[][] => 
  (output || []).map((entry: any) => 
    deserializeAws_restXmlStringList(
      getArrayIfSingleItem(entry["member"]),
      context
    )
  );

[kstich]

I'm not convinced that exposing this is needed at this time. This TS code is generated from this Java code, which could be updated to remove the contents and wrappedItem vars.

[trivikr]

Any suggestion on how writer.write can be removed from inside handleUnnamedTargetWrapping (code)

Requirement: The formatted output for wrappedItem needs to be passed to getMemberVisitor

[kstich]

The content that's being handled there is what you want to migrate behind the getArrayIfSingleItem call, so it shouldn't need to do anything other than remove the .write() call and return the function invoke to be used as the updated data source:

private String handleUnnamedTargetWrapping(GenerationContext context, Shape target, String dataSource) {
    if (!deserializationReturnsArray(target)) {
        return dataSource;
    }

    // The XML parser will set one K:V for a member that could
    // return multiple entries but only has one.
    // Update the target element if we target another level of collection.
    String targetLocation = getUnnamedAggregateTargetLocation(context.getModel(), target);
    return String.format("getArrayIfSingleItem(%s[\"%s\"])", dataSource, targetLocation);
}

[trivikr]

Using String.format is what was tried in experimental commit and was explained under alternatives in issue description.

Is the recommendation to use String.format and replicate formatting for specific use cases over exposing formatter.format as an API on CodeWriter?

[mtdowling]

I don't think exposing format is a general purpose solution, and would expose some workflows that could leave the CodeWriter in an inconsistent state. The CodeWriter handles imports and dependency management for you. If you use stuff like String#format then it doesn't do any of that, so this doesn't seem like a good option. If you use CodeWriter#format, then it would add dependencies and imports to the CodeWriter, but unless you actually call write at some point, the dependencies and imports might not actually be needed.

Have you considered just delegating to other methods and mutating the CodeWriter rather than returning a value?

[trivikr]

I don't think exposing format is a general purpose solution

I agree as CodeWriter is supposed to write code as per name, and not return something. This issue is cut to learn recommendation from Smithy team for this use case.

Have you considered just delegating to other methods and mutating the CodeWriter rather than returning a value?

The generic use case is ability to send output of formatter.format inside a method for further processing. How can delegation or mutation will help here?