Validate feature branch rollup #2141

marron-at-work · 2023-06-30T23:33:28Z

Merge the changes in PR #2099 and #2100.

Updates per comments on:

\in uses #[...]
validate now has optional name -- uses syntax validate exp as name; where the KW as denote the alias we are introducing for the validate
TS grammar file is updated

Open issues:

Need to finish support in printer.ts not sure how to add validates printing there. Any pointers would be appreciated.

…t#2137) The emitter framework did not handle enum member references. This PR adds support for them in the emitter framework (following a similar pattern to model references). It also adds support for them in the JSON Schema emitter, which simply emits the value inline (rather than doing any magic with $ref + json paths, which seems overkill). Fixes microsoft#2135.

github-actions · 2023-06-30T23:33:42Z

Changes in this PR will be published to the following url to try(check status of TypeSpec Pull Request Try It pipeline for publish status):
Playground: https://cadlplayground.z22.web.core.windows.net/prs/2141/

Website: https://cadlwebsite.z1.web.core.windows.net/prs/2141/

bterlson · 2023-06-30T23:50:54Z

I liked the syntactic of named validations previously, the as seems like it would get lost at the end and also is in a position that makes me worry about future ambiguities. Can you say more about why the change?

bterlson · 2023-06-30T23:51:28Z

Did the grammar changes make it into this PR?

marron-at-work · 2023-06-30T23:59:54Z

I made a few grammar changes should be in tmlanguage.ts. I am not very confident on this bit so it needs checking and a careful review (also the printer).

Two reasons for trying the alternative syntax:

I liked the validate name: exp; but didn't quite like how it looked like a property decl when it really is something different. Felt like the as made the focus on the logic and then the fact we were binding a name to it in addition.
This structure avoids the need to do a 2-token lookahead to disambiguate validate b: a > c; and the case validate b > c;

I don't think this is critical but wanted to try an alternative to see how it felt.

bterlson · 2023-07-03T19:53:48Z

Reflecting over the weekend I feel pretty strongly that validate name: exp is the right form here. While I agree it looks messy, I think we want to be consistent with the syntactic forms that introduce bindings in the scope of a declaration.

I'll try to get to looking at the checker implementation today but Wednesday is looking more likely at this point...

marron-at-work · 2023-07-04T06:22:02Z

Sounds good. Can definitely go with the validate name: exp form :)

I will need to do a 1-token lookahead for this though. I didn't see an obvious peek-ahead method through. Did I miss it or will I need to implement one?

packages/compiler/src/core/checker.ts

bterlson · 2023-07-05T20:09:43Z

packages/compiler/src/core/checker.ts

+ const name = vv.id?.sv ?? "[Anonymous]";
+
+ //
+ //TODO: typecheck the projection expression here or leave that problem for the emitter?


One thing I think we should check, at least: validate that the methods you invoke are present. We should have a standard built-in set of operations on types, it should be an error to say like arr::lenght. In the future we could discuss having some kind of extensibility mechanism here but I would like to start as locked-down as possible.

Maybe we can do a simplified check -- just that names exist/are in scope. Let's see if we want to go with a full checked expression AST (as outlined below) or just a simpler pass.

bterlson · 2023-07-05T20:19:27Z

packages/compiler/src/core/types.ts

@@ -285,6 +291,14 @@ export interface ModelProperty extends BaseType, DecoratedType {
 model?: Model;
 }

+export interface ModelValidate extends BaseType, DecoratedType {
+ kind: "ModelValidate";
+ node: ModelValidateNode;


I guess with this implementation we expect emitters to grab the node and party on from there. I wonder if we should instead define a separate AST structure for validates expressions? Part of this is because we don't consider the AST to be "public API" and therefore heavily discourage usage of this structure in emitters. But that aside, this could have a few possible benefits:

1: potentially simpler structure (e.g. probably don't need the top-level node but its ProjectionExpression value, don't need node flags, don't need directives, etc.)
2: offer text ranges relative to just the expression as opposed to the entire TypeSpec source file
3: include useful things from the check phase e.g. method invocation can have an actual target value you can use for comparison so you don't need to chug on a member expression yourself

bterlson · 2023-07-05T20:26:48Z

Another thing that bears thinking about is what we will need in the emitter framework. Party on AST is all well and good but we should probably also offer some kind of emitter framework APIs for making it easier to consume the AST and convert it to some code in some programming language, and for establishing an interface that folks can use to distribute and consume code that can do this sort of thing. We probably want a few off-the-shelf libraries to convert validations into expressions that can be composed with whatever you're doing for class/struct emit.

Just theorizing, but I think a separate ValidationEmitter base class that can be plugged into a TypeEmitter. ValidationEmitter would ultimately be a visitor like TypeEmitter, but doesn't need all the context propagation stuff.

For questions on printer and formatter, I think waiting for @timotheeguerin to get back is the best bet. Should be in next week!

…crosoft#2144)

…itter framework bugs (microsoft#2143)

…times (microsoft#2147) The first time such schemas were emitted, `emitSourceFile` polluted its definition, so all subsequent references would include the pollution.

Also: 1. added options to bypass file generation 2. added pre and post message 3. per template version schema validation

fix [microsoft#2136](microsoft#2136)

This split up unrelated content in the cli into different files. This PR keeps it to a minimum of moving content around. I have a follow up where all actions should try to reuse the logger/diagnostics instead of managing errors themself.

@Maxitems

fix microsoft#2110 Makes sure ```tsp @minItems(1) @Maxitems(5) model Endpoints is string[]; ``` Get applied correctly in openapi3.

…ion (microsoft#2181) fix microsoft#1960 (Discriminator docs) fix microsoft#1920 (Default interpretations) partial work for https://github.com/Azure/typespec-azure/issues/2960 ## Move discriminator doc to compiler Moved and expanded the small discriminator doc present in the http library. As discriminator is a compiler decorator and there is nothing specific about http. https://cadlwebsite.z1.web.core.windows.net/prs/2181/next/standard-library/discriminated-types ## Add documentation on how types are encoded in http Describe the default behaviors as well as how to configure https://cadlwebsite.z1.web.core.windows.net/prs/2181/next/standard-library/http/encoding ## Add content negotiation docs Add general http docs on how to define different content negotiation https://cadlwebsite.z1.web.core.windows.net/prs/2181/next/standard-library/http/content-types --------- Co-authored-by: Brian Terlson <[email protected]>

includes: 1. rest-api-spec repo project templates for ARM & DP 2. stand alone project templates for ARM & DP 3. Fixing a version comparison for template version

…soft#2202) Fix microsoft#2195.

Extracted this part of the breaking change from microsoft#2157 With prettier 3.0.0 the formatter is async. Which requires the emitter framework to provide a way to deal with async content mutation. This only breaks in the emitter framework is calling `this.emitter.emitSourceFile` directly which would happen if you wrote a custom `writeOutput`. If usage only include `sourceFile` override then it still allows returning a non promise. ```diff -const emittedSf = this.emitter.emitSourceFile(sf); +const emittedSf = await this.emitter.emitSourceFile(sf); ```

JS Ref docs was a mess, very hard to read with a lot of duplicate navigation that docusaurus was already providing. This PR change the following: - Remove the extra heading information - Make properties show up as a table instead of many different heading - Navigation is flattening all the types instead of having a sub folder per type (interface, enum, function, etc.) - This one can be a little more arguable that we don't want Before <img width="628" alt="image" src="https://github.com/microsoft/typespec/assets/1031227/bf49646b-6631-490f-8938-9b23e1e81e56"> After <img width="1340" alt="image" src="https://github.com/microsoft/typespec/assets/1031227/647c418f-61a0-4d20-bbbc-3f852fc308b9">

…idate_pr1

…#2209) fix [microsoft#2190](microsoft#2190) Add a few more tests as well to the cli/config -> compiler options resolution

…osoft#2208) Those tests were missing and because the rule tester runs outside of the program flow you can't test diagnostics in there. So adding those tests to make sure this work when integrated

marron-at-work · 2023-07-26T15:17:41Z

@nguerrera just pushed a commit to the pr branch that adds pretty printer support. Please take a quick look.

marron-at-work · 2023-07-26T15:20:59Z

One new open issue is that this PR introduces in as a keyword. However a model in auth.tsp uses this as a property name :(

Not sure if we can change this or we need to take some other action.

packages/compiler/src/formatter/print/printer.ts

timotheeguerin · 2023-07-26T15:33:45Z

packages/compiler/src/formatter/print/printer.ts

- doc.push(regularLine);
- if (shouldWrapInNewLines) {
- newLineBeforeNextProp = true;
+ if (propct > 0) {


probably don't need this extra check as nothing will get iterated in both those path.each

This makes sure there are properties/validates -- otherwise the node may not have the property (it is nullable) and then the path.each fails.

oh I see, should we change then to test for that propct !== undefined?

packages/compiler/src/formatter/print/printer.ts

marron-at-work · 2023-07-28T18:25:57Z

Hi @timotheeguerin thanks for the comments on the printer. I just pushed revisions based on them.

bterlson · 2023-07-28T18:44:22Z

It is unfortunate that we didn't reserve in. I see two options: 1, contextually reserve in validates expressions, or 2, as part of this PR allow members to be keywords (which I think is not at all ambiguous). Will discuss.

timotheeguerin · 2023-07-28T19:36:12Z

Hi @timotheeguerin thanks for the comments on the printer. I just pushed revisions based on them.

looks great, just replied to one but not blocking

Implement checking of validation clauses

bterlson and others added 2 commits June 30, 2023 11:31

Validate feature branch rollup

e32c516

marron-at-work requested review from bterlson, daviwil, nguerrera, markcowl, allenjzhang and timotheeguerin as code owners June 30, 2023 23:33

Remove myself from CODEOWNERS (microsoft#2149)

5764672

bterlson reviewed Jul 5, 2023

View reviewed changes

packages/compiler/src/core/checker.ts Show resolved Hide resolved

bterlson reviewed Jul 5, 2023

View reviewed changes

bterlson added 3 commits July 5, 2023 20:58

Export various emitter-related types from the JSON Schema package (mi…

874cceb

…crosoft#2144)

Implement JSON Schema Record<unknown>, Record<A | B>, and fix many em…

9f055a1

…itter framework bugs (microsoft#2143)

Fix emit of bundled schemas when has references and is used multiple …

0080b50

…times (microsoft#2147) The first time such schemas were emitted, `emitSourceFile` polluted its definition, so all subsequent references would include the pollution.

bterlson mentioned this pull request Jul 5, 2023

Implementation of validate for Model Statement and Scalar. #2100

Closed

timotheeguerin and others added 7 commits July 5, 2023 14:41

Add release note for typescript minium version 5.0 (microsoft#2132)

2bd3c7a

Adding versioning check for TypeSpec init template (microsoft#2121)

ab7c20e

Also: 1. added options to bypass file generation 2. added pre and post message 3. per template version schema validation

Fix: getNameType for union should include namespace (microsoft#2160)

95e2088

fix [microsoft#2136](microsoft#2136)

Add release notes for July release (microsoft#2164)

1df6ea3

Prepare typespec publish for July release (microsoft#2175)

140d6ce

Fix: Apply @minItems and @maxItems on model array (microsoft#2170)

09c2561

fix microsoft#2110 Makes sure ```tsp @minItems(1) @Maxitems(5) model Endpoints is string[]; ``` Get applied correctly in openapi3.

timotheeguerin and others added 11 commits July 13, 2023 21:51

Update tsp template to latest version (microsoft#2192)

7ac833a

includes: 1. rest-api-spec repo project templates for ARM & DP 2. stand alone project templates for ARM & DP 3. Fixing a version comparison for template version

Update @typespec/migrate to work with updated semver package (micro…

8648c2e

…soft#2202) Fix microsoft#2195.

Merge branch 'main' of https://github.com/microsoft/typespec into val…

be024d7

…idate_pr1

Fix: warn-as-error was ignored in the configuration file (microsoft…

656fda2

…#2209) fix [microsoft#2190](microsoft#2190) Add a few more tests as well to the cli/config -> compiler options resolution

Add tests making sure linter integrate with program diagnostics (micr…

8c8ab58

…osoft#2208) Those tests were missing and because the rule tester runs outside of the program flow you can't test diagnostics in there. So adding those tests to make sure this work when integrated

syntax to prefered

7c5b12a

Merge branch 'microsoft:main' into validate_pr1

bbedfc1

printer

65bc9c4

timotheeguerin reviewed Jul 26, 2023

View reviewed changes

printer tidy

0934f0a

bterlson and others added 12 commits July 28, 2023 18:49

Implement checking of validation expressions

f7951a4

Implement member expressions

4d4df0a

Implement call expressions and built-in methods

d384f9a

Implement completions of members inside validates clauses

0a05c6f

Implement meta member completions

8eaff4c

Implement completions of property references inside validates

a32c350

Produce subtype-reduced union type for if expressions

41d25ec

Create op types for lambdas (WIP), fix bugs

e821371

Merge pull request #1 from bterlson/validate_pr1

8760c59

Implement checking of validation clauses

a test

e2a938e

adding tests

afde41e

test

d589d02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate feature branch rollup #2141

Validate feature branch rollup #2141

marron-at-work commented Jun 30, 2023 •

edited

github-actions bot commented Jun 30, 2023

bterlson commented Jun 30, 2023

bterlson commented Jun 30, 2023

marron-at-work commented Jun 30, 2023

bterlson commented Jul 3, 2023

marron-at-work commented Jul 4, 2023

bterlson Jul 5, 2023

marron-at-work Jul 26, 2023

bterlson Jul 5, 2023

bterlson commented Jul 5, 2023

marron-at-work commented Jul 26, 2023

marron-at-work commented Jul 26, 2023

timotheeguerin Jul 26, 2023

marron-at-work Jul 28, 2023

timotheeguerin Jul 28, 2023

marron-at-work commented Jul 28, 2023

bterlson commented Jul 28, 2023

timotheeguerin commented Jul 28, 2023

Validate feature branch rollup #2141

Are you sure you want to change the base?

Validate feature branch rollup #2141

Conversation

marron-at-work commented Jun 30, 2023 • edited

github-actions bot commented Jun 30, 2023

bterlson commented Jun 30, 2023

bterlson commented Jun 30, 2023

marron-at-work commented Jun 30, 2023

bterlson commented Jul 3, 2023

marron-at-work commented Jul 4, 2023

bterlson Jul 5, 2023

Choose a reason for hiding this comment

marron-at-work Jul 26, 2023

Choose a reason for hiding this comment

bterlson Jul 5, 2023

Choose a reason for hiding this comment

bterlson commented Jul 5, 2023

marron-at-work commented Jul 26, 2023

marron-at-work commented Jul 26, 2023

timotheeguerin Jul 26, 2023

Choose a reason for hiding this comment

marron-at-work Jul 28, 2023

Choose a reason for hiding this comment

timotheeguerin Jul 28, 2023

Choose a reason for hiding this comment

marron-at-work commented Jul 28, 2023

bterlson commented Jul 28, 2023

timotheeguerin commented Jul 28, 2023

marron-at-work commented Jun 30, 2023 •

edited