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

http-client-java, improve error diagnostics for spotless failure #6300

Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -50,45 +50,52 @@ public class Main {

// java -jar target/azure-typespec-extension-jar-with-dependencies.jar
public static void main(String[] args) throws IOException {
// parameters
String inputYamlFileName = DEFAULT_OUTPUT_DIR + "code-model.yaml";
if (args.length >= 1) {
inputYamlFileName = args[0];
}
try {
// parameters
String inputYamlFileName = DEFAULT_OUTPUT_DIR + "code-model.yaml";
if (args.length >= 1) {
inputYamlFileName = args[0];
}

LOGGER.info("Code model file: {}", inputYamlFileName);

LOGGER.info("Code model file: {}", inputYamlFileName);
// load code-model.yaml
CodeModel codeModel = loadCodeModel(inputYamlFileName);

// load code-model.yaml
CodeModel codeModel = loadCodeModel(inputYamlFileName);
EmitterOptions emitterOptions = loadEmitterOptions(codeModel);

EmitterOptions emitterOptions = loadEmitterOptions(codeModel);
boolean sdkIntegration = true;
String outputDir = emitterOptions.getOutputDir();
Path outputDirPath = Paths.get(outputDir);
if (Files.exists(outputDirPath)) {
if (emitterOptions.getArm()) {
// check ../../parents/azure-client-sdk-parent
sdkIntegration = Files.exists(Paths.get(outputDir, "../../parents/azure-client-sdk-parent"));
} else {
try (Stream<Path> filestream = Files.list(outputDirPath)) {
Set<String> filenames = filestream.map(p -> p.getFileName().toString())
.map(name -> name.toLowerCase(Locale.ROOT))
.collect(Collectors.toSet());

// if there is already pom and source, do not overwrite them (includes README.md, CHANGELOG.md
// etc.)
sdkIntegration = !filenames.containsAll(Arrays.asList("pom.xml", "src"));
}
}
}

boolean sdkIntegration = true;
String outputDir = emitterOptions.getOutputDir();
Path outputDirPath = Paths.get(outputDir);
if (Files.exists(outputDirPath)) {
if (emitterOptions.getArm()) {
// check ../../parents/azure-client-sdk-parent
sdkIntegration = Files.exists(Paths.get(outputDir, "../../parents/azure-client-sdk-parent"));
handleFluent(codeModel, emitterOptions, sdkIntegration);
} else {
try (Stream<Path> filestream = Files.list(outputDirPath)) {
Set<String> filenames = filestream.map(p -> p.getFileName().toString())
.map(name -> name.toLowerCase(Locale.ROOT))
.collect(Collectors.toSet());

// if there is already pom and source, do not overwrite them (includes README.md, CHANGELOG.md etc.)
sdkIntegration = !filenames.containsAll(Arrays.asList("pom.xml", "src"));
}
handleDPG(codeModel, emitterOptions, sdkIntegration, outputDir);
}
}

if (emitterOptions.getArm()) {
handleFluent(codeModel, emitterOptions, sdkIntegration);
} else {
handleDPG(codeModel, emitterOptions, sdkIntegration, outputDir);
// ensure the process exits as expected
System.exit(0);
} catch (Exception e) {
LOGGER.error("Unhandled error.", e);
System.exit(1);
}
// ensure the process exits as expected
System.exit(0);
}

private static void handleFluent(CodeModel codeModel, EmitterOptions emitterOptions, boolean sdkIntegration) {
Expand Down
Loading
Loading