Skip to content

Commit

Permalink
http-client-java, improve error diagnostics for spotless failure (#6300)
Browse files Browse the repository at this point in the history
The failure is usually due to Java syntax problem. We want to print the
file name and content for better diagnostics.

Sample output of this PR.

```
[main] ERROR com.microsoft.typespec.http.client.generator.TypeSpecPlugin - Failed to format file 'src/main/java/azure/clientgenerator/core/usage/implementation/ModelInOperationsImpl.java'
177:15: error: illegal unicode escape
174 
175     /**
176      * Serialize the 'OrphanModel' as request body.
177      * data\userdomain
178      * 
179      * Expected body parameter: 
180      * ```json
```

I've also add logic to parse the JAR output to diagnostics/trace for
tsp.


![image](https://github.com/user-attachments/assets/180cebb0-de05-4094-8820-7d34928d7615)


![image](https://github.com/user-attachments/assets/4b8b12e9-1bad-489e-bdeb-93ff485f9347)

autorest.java Azure/autorest.java#3035
  • Loading branch information
weidongxu-microsoft authored Mar 7, 2025
1 parent 5df4498 commit 94a9b9e
Show file tree
Hide file tree
Showing 10 changed files with 190 additions and 90 deletions.
8 changes: 7 additions & 1 deletion packages/http-client-java/emitter/src/code-model-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2472,8 +2472,14 @@ export class CodeModelBuilder {

private processObjectSchema(type: SdkModelType, name: string): ObjectSchema {
const rawModelType = type.__raw;
if (!name && !type.name) {
reportDiagnostic(this.program, {
code: "empty-name",
target: rawModelType ?? NoTarget,
});
}
const namespace = getNamespace(rawModelType);
const objectSchema = new ObjectSchema(name, type.doc ?? "", {
const objectSchema = new ObjectSchema(type.name ?? name, type.doc ?? "", {
summary: type.summary,
language: {
default: {
Expand Down
73 changes: 66 additions & 7 deletions packages/http-client-java/emitter/src/emitter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { EmitContext, getNormalizedAbsolutePath, NoTarget, resolvePath } from "@typespec/compiler";
import {
EmitContext,
getNormalizedAbsolutePath,
NoTarget,
Program,
resolvePath,
} from "@typespec/compiler";
import { promises } from "fs";
import { dump } from "js-yaml";
import { dirname } from "path";
Expand Down Expand Up @@ -106,26 +112,28 @@ export async function $onEmit(context: EmitContext<EmitterOptions>) {
javaArgs.push(codeModelFileName);
try {
const result = await spawnAsync("java", javaArgs, { stdio: "pipe" });
trace(program, `Code generation log: ${result.stdout}`);
reportJarOutput(program, result.stdout);
// trace(program, `Code generation log: ${result.stdout}`);
} catch (error: any) {
if (error && "code" in error && error["code"] === "ENOENT") {
reportDiagnostic(program, {
code: "invalid-java-sdk-dependency",
target: NoTarget,
});
} else {
if (error instanceof SpawnError) {
reportJarOutput(program, error.stdout);
// trace(program, `Code generation log: ${error.stdout}`);
}

// error in Java codegen, report as unknown error
reportDiagnostic(program, {
code: "unknown-error",
format: {
errorMessage: `The emitter was unable to generate client code from this TypeSpec, please open an issue on https://github.com/microsoft/typespec, include TypeSpec source and all the diagnostic information in your submission.\nOutput: ${error.stdout}\nError: ${error.stderr}`,
errorMessage: `The emitter was unable to generate client code from this TypeSpec, please open an issue on https://github.com/microsoft/typespec, include TypeSpec source and all the diagnostic information in your submission.`,
},
target: NoTarget,
});
if (error instanceof SpawnError) {
trace(program, `Code generation log: ${error.stdout}`);
trace(program, `Code generation error: ${error.stderr}`);
}
}
}

Expand All @@ -135,3 +143,54 @@ export async function $onEmit(context: EmitContext<EmitterOptions>) {
}
}
}

function reportJarOutput(program: Program, jarOutput: string) {
const lines = jarOutput.split("\n");
const logs: Array<string> = [];

// parse stdout to array of logs
let currentLog = undefined;
for (const line of lines) {
if (
line.startsWith("TRACE ") ||
line.startsWith("DEBUG ") ||
line.startsWith("INFO ") ||
line.startsWith("WARN ") ||
line.startsWith("ERROR ")
) {
if (currentLog) {
logs.push(currentLog);
}
currentLog = line;
} else if (currentLog) {
currentLog = currentLog + "\n" + line;
}
}
if (currentLog) {
logs.push(currentLog);
}

// trace or report the logs, according to log level
for (const log of logs) {
if (log.startsWith("ERROR ")) {
reportDiagnostic(program, {
code: "generator-error",
format: {
errorMessage: log.substring(6),
},
target: NoTarget,
});
} else if (log.startsWith("WARN ")) {
reportDiagnostic(program, {
code: "generator-warning",
format: {
warningMessage: log.substring(5),
},
target: NoTarget,
});
} else {
const index = log.indexOf(" ");
trace(program, log.substring(index + 1));
}
}
}
18 changes: 18 additions & 0 deletions packages/http-client-java/emitter/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ export const $lib = createTypeSpecLibrary({
default: paramMessage`An unknown error occurred. ${"errorMessage"}`,
},
},
"generator-error": {
severity: "error",
messages: {
default: paramMessage`${"errorMessage"}`,
},
},
"invalid-java-sdk-dependency": {
severity: "error",
messages: {
Expand Down Expand Up @@ -130,8 +136,20 @@ export const $lib = createTypeSpecLibrary({
multipartFormData: paramMessage`Unrecognized type for multipart form data, kind '${"typeKind"}'.`,
},
},
"empty-name": {
severity: "error",
messages: {
default: "Name from TCGC is empty.",
},
},

// warning
"generator-warning": {
severity: "warning",
messages: {
default: paramMessage`${"warningMessage"}`,
},
},
"no-service": {
severity: "warning",
messages: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public static void writeToFiles(Map<String, String> javaFiles, NewPlugin plugin,
}

try {
CodeFormatterUtil.formatCode(javaFiles, plugin);
CodeFormatterUtil.formatCode(javaFiles, plugin, logger);
} catch (Exception ex) {
throw new RuntimeException(ex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,62 @@
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.slf4j.Logger;

/**
* Utility class that handles code formatting.
*/
public final class CodeFormatterUtil {

private static final Pattern SPOTLESS_ERROR_PATTERN
= Pattern.compile("^(\\d+):\\d+: error: (.*)$", Pattern.MULTILINE);
private static final int SPOTLESS_FILE_CONTENT_RANGE = 3;

/**
* Formats the given files by removing unused imports and applying Eclipse code formatting.
*
* @param files The files to format.
* @param plugin The plugin to use to write the formatted files.
*/
public static void formatCode(Map<String, String> files, NewPlugin plugin) {
for (Map.Entry<String, String> file : formatCodeInternal(files.entrySet())) {
plugin.writeFile(file.getKey(), file.getValue(), null);
public static void formatCode(Map<String, String> files, NewPlugin plugin, Logger logger) {
try {
for (Map.Entry<String, String> file : formatCodeInternal(files.entrySet())) {
plugin.writeFile(file.getKey(), file.getValue(), null);
}
} catch (SpotlessException ex) {
// format one file at a time, to give better error diagnostics
for (Map.Entry<String, String> file : files.entrySet()) {
try {
formatCodeInternal(List.of(file));
} catch (RuntimeException e) {
// by default, log the whole file
String content = file.getValue();

// if we can find the line number from the error message, refine the "content" to the part of file
// around the line
Matcher matcher = SPOTLESS_ERROR_PATTERN.matcher(e.getMessage());
if (matcher.find()) {
int lineNumber = Integer.parseInt(matcher.group(1));

StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append(matcher.group(0)).append("\n");

// line number from log starts from 1
String[] lines = content.split("\n");
int lineIndexBegin = Math.max(0, lineNumber - 1 - SPOTLESS_FILE_CONTENT_RANGE);
int lineIndexEnd = Math.min(lines.length - 1, lineNumber - 1 + SPOTLESS_FILE_CONTENT_RANGE);
for (int lineIndex = lineIndexBegin; lineIndex <= lineIndexEnd; ++lineIndex) {
stringBuilder.append(lineIndex + 1).append(" ").append(lines[lineIndex]).append("\n");
}
content = stringBuilder.toString();
}
logger.error("Failed to format file '{}'\n{}", file.getKey(), content);
}
}
throw ex;
}
}

Expand All @@ -41,10 +82,10 @@ public static void formatCode(Map<String, String> files, NewPlugin plugin) {
*
* @param files The files to format. The entry is filename and content.
* @return the files after format.
* @throws Exception If code formatting fails.
* @throws RuntimeException If code formatting fails.
*/
public static List<String> formatCode(Collection<Map.Entry<String, String>> files) throws Exception {
return formatCodeInternal(files).stream().map(Map.Entry::getValue).collect(Collectors.toList());
public static List<String> formatCode(Map<String, String> files) {
return formatCodeInternal(files.entrySet()).stream().map(Map.Entry::getValue).collect(Collectors.toList());
}

private static List<Map.Entry<String, String>> formatCodeInternal(Collection<Map.Entry<String, String>> files) {
Expand Down Expand Up @@ -101,12 +142,18 @@ private static void attemptMavenSpotless(Path pomPath) {

if (process.isAlive() || process.exitValue() != 0) {
process.destroyForcibly();
throw new RuntimeException(
"Spotless failed to complete within 300 seconds or failed with an error code. "
throw new SpotlessException(
"Spotless failed to complete within 300 seconds or failed with an error code. Output:\n"
+ Files.readString(outputFile.toPath()));
}
} catch (IOException | InterruptedException ex) {
throw new RuntimeException("Failed to run Spotless on generated code.", ex);
}
}

private static final class SpotlessException extends RuntimeException {
public SpotlessException(String message) {
super(message);
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ public String write(List<FluentExample> examples, List<JavaFile> sampleJavaFiles
assert examples.size() == sampleJavaFiles.size();

// clean up copyright etc.
List<Map.Entry<String, String>> javaFiles = sampleJavaFiles.stream()
.map(e -> Map.entry(e.getFilePath(), cleanJavaFile(e)))
.collect(Collectors.toList());
Map<String, String> javaFiles
= sampleJavaFiles.stream().collect(Collectors.toMap(JavaFile::getFilePath, SampleTemplate::cleanJavaFile));
// format code
List<String> javaFileContents;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ enum ApiVersions {
op RpcOperationWithAdditionalResponse<
TParams,
TResponse extends TypeSpec.Reflection.Model,
TAdditionalResponse extends object,
Traits extends object = {},
TAdditionalResponse extends {},
Traits extends {} = {},
TErrorResponse = Azure.Core.Foundations.ErrorResponse
> is Foundations.Operation<
TParams & Azure.Core.Traits.Private.TraitProperties<Traits, TraitLocation.Parameters>,
Expand Down
Loading

0 comments on commit 94a9b9e

Please sign in to comment.