Skip to content

Additional testing and fixes related to Swagger provider#150

Merged
dsyme merged 2 commits intofsprojects:masterfrom
dsyme:tmp3
Oct 20, 2017
Merged

Additional testing and fixes related to Swagger provider#150
dsyme merged 2 commits intofsprojects:masterfrom
dsyme:tmp3

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Oct 20, 2017

The Swagger provider does some interesting things like using one generated type as the base type for other generated types. This causes lookups on the types implied by the generated assembly before generation is complete.

This PR adjusts the translation implementation of the SDK to allow for this, and adds a specific test case for this

@dsyme dsyme merged commit 27232ac into fsprojects:master Oct 20, 2017
let simpleName = Path.GetFileNameWithoutExtension(assemblyFileName)
ProvidedAssembly(AssemblyName(simpleName), assemblyFileName)

new (assemblyName, assemblyFileName) =
Copy link
Member

Choose a reason for hiding this comment

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

Previously, when passing a filename, you didn't need to specify the assemblyname - It seems like this is almost always going to be Path.GetFileNameWithoutExtension off the assemblyFileName - should it just be taken care of for the caller, so they don't need to build the assembly name as well? (That's closer to the old API - you just passed one string for path)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's now a parameterless constructor for ProvidedAssembly https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/master/src/ProvidedTypes.fsi#L400 that generates a temporary name - is that ok?

Copy link
Member

Choose a reason for hiding this comment

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

I like the parameterless constructor, but I do not like the implementation of Path.GetTempFileName

Creates a uniquely named, zero-byte temporary file on disk and returns the full path of that file.

The GetTempFileName method will raise an IOException if it is used to create more than 65535 files without deleting previous temporary files.

Once, I already wrote the code that calls Path.GetTempFileName too often and did not delete this empty file... This is weird.

Not sure if it is the case... But in theory, TP may crash in looong running VS/IDE instance 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sergey-tihon Yes, you're correct, we are creating empty files, that is wrong

@Thorium
Copy link
Member

Thorium commented Oct 21, 2017

A parameter-less constructor would be nice:

ProvidedConstructor([ProvidedParameter("p", typeof<int>)])

Current workaround:

let empty = fun (_:Expr list) -> <@@ () @@> // <-- some ascii art
ProvidedConstructor([(* ... *)], empty)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants