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

Meta: integrate esmeta type checker into CI #2926

Merged
merged 1 commit into from
Nov 2, 2022
Merged

Conversation

michaelficarra
Copy link
Member

@michaelficarra michaelficarra commented Oct 5, 2022

Currently has 72 false positives. Waiting on:

@michaelficarra michaelficarra added meta editor call to be discussed in the next editor call and removed editor call to be discussed in the next editor call labels Oct 5, 2022
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't be made to run locally with npm install && npm run something?

.github/workflows/esmeta-typecheck.yml Outdated Show resolved Hide resolved
Comment on lines 21 to 22
- name: download esmeta
run: 'git clone --depth 1 --branch v0.1.0-rc3 https://github.com/es-meta/esmeta.git "${ESMETA_HOME}"'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this can be done with the checkout action?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that better though? This says exactly what's going on.

@bakkot
Copy link
Contributor

bakkot commented Oct 12, 2022

For exclusions support, is there an error if something excluded does not have an error? We'll never remember to remove stuff from it otherwise.

@michaelficarra
Copy link
Member Author

@bakkot No, it doesn't look like it: es-meta/esmeta@271fcae. I'll ask them to change it to xfail semantics.

@jhnaldo
Copy link
Contributor

jhnaldo commented Oct 18, 2022

Hi, @bakkot. Do you mean that you want ESMeta to throw an error when excluded algorithms do not have any type mismatches?

@bakkot
Copy link
Contributor

bakkot commented Oct 20, 2022

@jhnaldo Yes, that's what I mean.

Specifically, ideally, if there's any line in the "excluded" file such that the build would succeed without that line being present, then the build should fail. In other words, rather than actually excluding anything, it should run the full build and then assert that the set of algorithms with errors is exactly the set of algorithms listed in the exclusions file, rather than being a subset of that list.

@jhnaldo
Copy link
Contributor

jhnaldo commented Oct 23, 2022

@bakkot Thank you for the kind explanation! I supported the feature you mentioned to detect unused names listed in the "excluded" file in the ESMeta v0.1.0-rc5 version. And, I attached the tight esmeta-ignore.json file as follows:

[
  "AddEntriesFromIterable",
  "AsyncGeneratorBody[0,0].EvaluateAsyncGeneratorBody",
  "BoundFunctionCreate",
  "Catch[1,0].CatchClauseEvaluation",
  "ClassElement[0,0].ClassElementEvaluation",
  "ClassElement[1,0].ClassElementEvaluation",
  "ClassStaticBlockBody[0,0].EvaluateClassStaticBlockBody",
  "CreateArrayIterator",
  "CreateBuiltinFunction",
  "CreateDynamicFunction",
  "CreateMappedArgumentsObject",
  "DeclarativeEnvironmentRecord.InitializeBinding",
  "DeclarativeEnvironmentRecord.SetMutableBinding",
  "FlattenIntoArray",
  "FunctionBody[0,0].EvaluateFunctionBody",
  "GeneratorBody[0,0].EvaluateGeneratorBody",
  "GeneratorResumeAbrupt",
  "GeneratorValidate",
  "GetGlobalObject",
  "GetMethod",
  "GetNewTarget",
  "GetPromiseResolve",
  "GetPrototypeFromConstructor",
  "GetThisValue",
  "INTRINSICS.DefaultTimeZone",
  "InnerModuleEvaluation",
  "IntegerIndexedObjectCreate",
  "LabelledItem[1,0].LabelledEvaluation",
  "LengthOfArrayLike",
  "MethodDefinition[0,0].DefineMethod",
  "ModuleNamespaceCreate",
  "NewPromiseReactionJob",
  "NormalCompletion",
  "OrdinaryGet",
  "PrivateGet",
  "ProxyCreate",
  "RegExpInitialize",
  "SerializeJSONObject",
  "SerializeJSONProperty",
  "SetDefaultGlobalBindings",
  "SetViewValue",
  "SortIndexedProperties",
  "SourceTextModuleRecord.ResolveExport",
  "SpeciesConstructor",
  "Statement[0,0].LabelledEvaluation",
  "Statement[1,0].LabelledEvaluation",
  "Statement[2,0].LabelledEvaluation",
  "StringCreate",
  "StringPad",
  "TemplateString",
  "ToBigInt64",
  "ToBigUint64",
  "ToUint32",
  "TrimString",
  "WeakRefDeref"
]

@michaelficarra michaelficarra added editor call to be discussed in the next editor call and removed editor call to be discussed in the next editor call labels Oct 23, 2022
@michaelficarra michaelficarra marked this pull request as ready for review October 26, 2022 22:17
@michaelficarra michaelficarra requested a review from a team October 26, 2022 22:20
@michaelficarra michaelficarra added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Nov 1, 2022
@michaelficarra michaelficarra changed the title WIP: integrate esmeta type checker into CI Meta: integrate esmeta type checker into CI Nov 1, 2022
@ljharb ljharb force-pushed the esmeta-ci-typechecking branch from 76d9490 to b48c1e8 Compare November 2, 2022 22:15
@ljharb ljharb force-pushed the esmeta-ci-typechecking branch from b48c1e8 to a619745 Compare November 2, 2022 22:22
@ljharb ljharb merged commit a619745 into main Nov 2, 2022
@ljharb ljharb deleted the esmeta-ci-typechecking branch November 2, 2022 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants