Skip to content

Commit

Permalink
http-client-java, improve diagnostic (#5702)
Browse files Browse the repository at this point in the history 
link #5511

1. bump tcgc to 0.50.2
2. adopt part of the `serializationOptions` (not a full adoption due to
bug Azure/typespec-azure#2118)
3. refactor entry, now it is `index.ts`, which exposes the few types
needed by compiler
4. migrate to the standard diagnostic in typespec compiler
5. some refactor on error/warning code and message, group some to same
code, etc.
6. add `target` to diagnostic, when we can find it
  • Loading branch information
@weidongxu-microsoft
weidongxu-microsoft authored Jan 24, 2025
1 parent e477a88 commit 030cba0
Show file tree
Hide file tree
Showing 33 changed files with 646 additions and 344 deletions.
165 changes: 99 additions & 66 deletions packages/http-client-java/emitter/src/code-model-builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,17 @@ import {
SdkUnionType,
createSdkContext,
getAllModels,
getWireName,
isApiVersion,
isSdkBuiltInKind,
isSdkIntKind,
} from "@azure-tools/typespec-client-generator-core";
import {
EmitContext,
Interface,
Model,
ModelProperty,
Namespace,
NoTarget,
Operation,
Program,
Type,
Expand All @@ -99,13 +100,7 @@ import {
HttpStatusCodesEntry,
Visibility,
getAuthentication,
getHeaderFieldName,
getPathParamName,
getQueryParamName,
isCookieParam,
isHeader,
isPathParam,
isQueryParam,
} from "@typespec/http";
import { getSegment } from "@typespec/rest";
import { getAddedOnVersions } from "@typespec/versioning";
Expand All @@ -120,8 +115,8 @@ import { ConstantSchema, ConstantValue } from "./common/schemas/constant.js";
import { OrSchema } from "./common/schemas/relationship.js";
import { DurationSchema } from "./common/schemas/time.js";
import { SchemaContext, SchemaUsage } from "./common/schemas/usage.js";
import { EmitterOptions } from "./emitter.js";
import { createPollOperationDetailsSchema, getFileDetailsSchema } from "./external-schemas.js";
import { EmitterOptions, createDiagnostic, reportDiagnostic } from "./lib.js";
import { ClientContext } from "./models.js";
import {
CONTENT_TYPE_KEY,
Expand All @@ -148,9 +143,8 @@ import {
pushDistinct,
} from "./type-utils.js";
import {
DiagnosticError,
getNamespace,
logError,
logWarning,
pascalCase,
removeClientSuffix,
stringArrayContainsIgnoreCase,
Expand Down Expand Up @@ -193,7 +187,10 @@ export class CodeModelBuilder {

const service = listServices(this.program)[0];
if (!service) {
this.logWarning("TypeSpec for HTTP client should define a service.");
reportDiagnostic(this.program, {
code: "no-service",
target: NoTarget,
});
}
this.serviceNamespace = service?.type ?? this.program.getGlobalNamespaceType();

Expand Down Expand Up @@ -259,7 +256,7 @@ export class CodeModelBuilder {
// TODO: it is not very likely, but different client could have different auth
const auth = getAuthentication(this.program, this.serviceNamespace);
if (auth) {
this.processAuth(auth);
this.processAuth(auth, this.serviceNamespace);
}

if (this.sdkContext.arm) {
Expand Down Expand Up @@ -315,7 +312,7 @@ export class CodeModelBuilder {
return hostParameters;
}

private processAuth(auth: Authentication) {
private processAuth(auth: Authentication, serviceNamespace: Namespace | Interface | Operation) {
const securitySchemes: SecurityScheme[] = [];
for (const option of auth.options) {
for (const scheme of option.schemes) {
Expand All @@ -332,7 +329,11 @@ export class CodeModelBuilder {
securitySchemes.push(oauth2Scheme);
} else {
// there is no TokenCredential in clientcore, hence use Bearer Authentication directly
this.logWarning(`OAuth2 auth scheme is generated as KeyCredential.`);
reportDiagnostic(this.program, {
code: "auth-scheme-not-supported",
messageId: "oauth2Unbranded",
target: serviceNamespace,
});

const keyScheme = new KeySecurityScheme({
name: "authorization",
Expand All @@ -351,9 +352,11 @@ export class CodeModelBuilder {
});
securitySchemes.push(keyScheme);
} else {
this.logWarning(
`ApiKey auth is currently only supported for ApiKeyLocation.header.`,
);
reportDiagnostic(this.program, {
code: "auth-scheme-not-supported",
messageId: "apiKeyLocation",
target: serviceNamespace,
});
}
}
break;
Expand All @@ -367,9 +370,12 @@ export class CodeModelBuilder {

if (this.isBranded()) {
// Azure would not allow BasicAuth or BearerAuth
this.logWarning(
`HTTP auth with ${scheme.scheme} scheme is not supported for Azure.`,
);
reportDiagnostic(this.program, {
code: "auth-scheme-not-supported",
messageId: "basicAuthBranded",
format: { scheme: scheme.scheme },
target: serviceNamespace,
});
continue;
}
}
Expand Down Expand Up @@ -561,7 +567,11 @@ export class CodeModelBuilder {
} else {
this.apiVersion = versions.find((it: string) => it === this.sdkContext.apiVersion);
if (!this.apiVersion) {
this.logError("Unrecognized api-version: " + this.sdkContext.apiVersion);
reportDiagnostic(this.program, {
code: "invalid-api-version",
format: { apiVersion: this.sdkContext.apiVersion },
target: NoTarget,
});
}
}

Expand Down Expand Up @@ -593,7 +603,10 @@ export class CodeModelBuilder {
}
}
} else if (initializationProperty.type.variantTypes.length > 2) {
this.logError("Multiple server URL defined for one client is not supported yet.");
reportDiagnostic(this.program, {
code: "multiple-server-not-supported",
target: initializationProperty.type.__raw ?? NoTarget,
});
}
} else if (initializationProperty.type.kind === "endpoint") {
sdkPathParameters = initializationProperty.type.templateArguments;
Expand Down Expand Up @@ -830,36 +843,51 @@ export class CodeModelBuilder {
let generateConvenienceApi: boolean = sdkMethod.generateConvenient;
let generateProtocolApi: boolean = sdkMethod.generateProtocol;

let apiComment: string | undefined = undefined;
let diagnostic = undefined;
if (generateConvenienceApi) {
// check if the convenience API need to be disabled for some special cases
if (operationIsMultipart(httpOperation)) {
// do not generate protocol method for multipart/form-data, as it be very hard for user to prepare the request body as BinaryData
generateProtocolApi = false;
apiComment = `Protocol API requires serialization of parts with content-disposition and data, as operation '${operationName}' is 'multipart/form-data'`;
this.logWarning(apiComment);
diagnostic = createDiagnostic({
code: "protocol-api-not-generated",
messageId: "multipartFormData",
format: { operationName: operationName },
target: sdkMethod.__raw ?? NoTarget,
});
this.program.reportDiagnostic(diagnostic);
} else if (operationIsMultipleContentTypes(httpOperation)) {
// and multiple content types
// issue link: https://github.com/Azure/autorest.java/issues/1958#issuecomment-1562558219
generateConvenienceApi = false;
apiComment = `Convenience API is not generated, as operation '${operationName}' is multiple content-type`;
this.logWarning(apiComment);
diagnostic = createDiagnostic({
code: "convenience-api-not-generated",
messageId: "multipleContentType",
format: { operationName: operationName },
target: sdkMethod.__raw ?? NoTarget,
});
this.program.reportDiagnostic(diagnostic);
} else if (
operationIsJsonMergePatch(httpOperation) &&
this.options["stream-style-serialization"] === false
) {
// do not generate convenient method for json merge patch operation if stream-style-serialization is not enabled
generateConvenienceApi = false;
apiComment = `Convenience API is not generated, as operation '${operationName}' is 'application/merge-patch+json' and stream-style-serialization is not enabled`;
this.logWarning(apiComment);
diagnostic = createDiagnostic({
code: "convenience-api-not-generated",
messageId: "jsonMergePatch",
format: { operationName: operationName },
target: sdkMethod.__raw ?? NoTarget,
});
this.program.reportDiagnostic(diagnostic);
}
}
if (generateConvenienceApi && convenienceApiName) {
codeModelOperation.convenienceApi = new ConvenienceApi(convenienceApiName);
}
if (apiComment) {
if (diagnostic) {
codeModelOperation.language.java = new Language();
codeModelOperation.language.java.comment = apiComment;
codeModelOperation.language.java.comment = diagnostic.message;
}

// check for generating protocol api or not
Expand Down Expand Up @@ -1081,11 +1109,12 @@ export class CodeModelBuilder {
lroMetadata.finalResponse.envelopeResult.properties?.forEach((p) => {
// TODO: "p.__raw?.name" should be "p.name", after TCGC fix https://github.com/Azure/typespec-azure/issues/2072
if (
!finalResultPropertySerializedName &&
p.kind === "property" &&
p.serializedName &&
p.serializationOptions.json?.name &&
p.__raw?.name === finalResultPropertyClientName
) {
finalResultPropertySerializedName = p.serializedName;
finalResultPropertySerializedName = p.serializationOptions.json?.name;
}
});
}
Expand Down Expand Up @@ -1247,7 +1276,11 @@ export class CodeModelBuilder {

default:
if (format) {
this.logWarning(`Unrecognized header parameter format: '${format}'.`);
reportDiagnostic(this.program, {
code: "header-parameter-format-not-supported",
format: { format: format },
target: param.__raw ?? NoTarget,
});
}
break;
}
Expand Down Expand Up @@ -1879,9 +1912,12 @@ export class CodeModelBuilder {
}
}
}
const errorMsg = `Unrecognized type: '${type.kind}'.`;
this.logError(errorMsg);
throw new Error(errorMsg);
const diagnostic = createDiagnostic({
code: "unrecognized-type",
format: { typeKind: type.kind },
target: type.__raw ?? NoTarget,
});
throw new DiagnosticError(diagnostic);
}

private processBuiltInType(type: SdkBuiltInType, nameHint: string): Schema {
Expand Down Expand Up @@ -2295,24 +2331,38 @@ export class CodeModelBuilder {
schema = this.processSchema(nonNullType, "");
}

const getPropertySerializedName = (property: SdkBodyModelPropertyType) => {
// TODO: remove the "property.serializedName" after bug https://github.com/microsoft/typespec/pull/5702 is fixed
return (
property.serializationOptions.json?.name ??
property.serializationOptions.multipart?.name ??
property.serializedName
);
};

return new Property(prop.name, prop.doc ?? "", schema, {
summary: prop.summary,
required: !prop.optional,
nullable: nullable,
readOnly: this.isReadOnly(prop),
serializedName: prop.kind === "property" ? prop.serializedName : undefined,
serializedName: prop.kind === "property" ? getPropertySerializedName(prop) : undefined,
extensions: extensions,
});
}

private processUnionSchema(type: SdkUnionType, name: string): Schema {
if (!(type.__raw && type.__raw.kind === "Union")) {
this.logError(`Invalid type for union: '${type.kind}'.`);
reportDiagnostic(this.program, {
code: "unrecognized-type",
messageId: "unionType",
format: { typeKind: type.kind },
target: type.__raw ?? NoTarget,
});
}
const rawUnionType: Union = type.__raw as Union;
const namespace = getNamespace(rawUnionType);
const baseName = type.name ?? pascalCase(name) + "Model";
this.logWarning(
this.trace(
`Convert TypeSpec Union '${getUnionDescription(rawUnionType, this.typeNameOptions)}' to Class '${baseName}'`,
);
const unionSchema = new OrSchema(baseName + "Base", type.doc ?? "", {
Expand Down Expand Up @@ -2360,7 +2410,7 @@ export class CodeModelBuilder {

private getUnionVariantName(type: Type | undefined, option: any): string {
if (type === undefined) {
this.logWarning("Union variant type is undefined.");
this.trace("Union variant type is undefined.");
return "UnionVariant";
}
switch (type.kind) {
Expand Down Expand Up @@ -2413,7 +2463,7 @@ export class CodeModelBuilder {
case "UnionVariant":
return (typeof type.name === "string" ? type.name : undefined) ?? "UnionVariant";
default:
this.logWarning(`Unrecognized type for union variable: '${type.kind}'.`);
this.trace(`Unrecognized type for union variable: '${type.kind}'.`);
return "UnionVariant";
}
}
Expand Down Expand Up @@ -2461,9 +2511,13 @@ export class CodeModelBuilder {
},
);
} else {
const errorMsg = `Invalid type for multipart form data: '${property.type.kind}'.`;
this.logError(errorMsg);
throw new Error(errorMsg);
const diagnostic = createDiagnostic({
code: "unrecognized-type",
messageId: "multipartFormData",
format: { typeKind: property.type.kind },
target: property.type.__raw ?? NoTarget,
});
throw new DiagnosticError(diagnostic);
}
}

Expand All @@ -2475,19 +2529,6 @@ export class CodeModelBuilder {
return target ? getSummary(this.program, target) : undefined;
}

private getSerializedName(target: ModelProperty): string {
if (isHeader(this.program, target)) {
return getHeaderFieldName(this.program, target);
} else if (isQueryParam(this.program, target)) {
return getQueryParamName(this.program, target);
} else if (isPathParam(this.program, target)) {
return getPathParamName(this.program, target);
} else {
// TODO: currently this is only for JSON
return getWireName(this.sdkContext, target);
}
}

private isReadOnly(target: SdkModelPropertyType): boolean {
const segment = target.__raw ? getSegment(this.program, target.__raw) !== undefined : false;
if (segment) {
Expand Down Expand Up @@ -2618,14 +2659,6 @@ export class CodeModelBuilder {
}
}

private logError(msg: string) {
logError(this.program, msg);
}

private logWarning(msg: string) {
logWarning(this.program, msg);
}

private trace(msg: string) {
trace(this.program, msg);
}
Expand Down
Loading

0 comments on commit 030cba0

Please sign in to comment.