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

Support all option types in the project file #4506

Conversation

keyboardDrummer
Copy link
Member

@keyboardDrummer keyboardDrummer commented Sep 1, 2023

Changes

  • Support all option types in the project file by parsing the project file options in the same way as the CLI parses them

Testing

  • Added a case that uses an enum option to an existing test

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

}
errorWriter.WriteLine(
$"Error: property '{tomlPath}' is of type '{tomlValue.GetType()}' but should be of type '{type}'");
var parseResult = option.Parse(new[] { option.Aliases.First(), tomlValue });
Copy link
Member Author

Choose a reason for hiding this comment

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

This will parse the project file options in the same way as the CLI parses them, so there won't be any discrepancy any more.

ghost function Bar(): int { 3 }".TrimStart();

var projectFileSource = @"
includes = [""**/*.dfy""]

[options]
warn-shadowing = false
quantifier-syntax = 4
Copy link
Member

Choose a reason for hiding this comment

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

In your test, could you please add an option that was otherwise translated to an enum or otherwise processed?
For example:
test-assumptions = externs
It serves no purpose for your example other than testing that the parsing mechanism works (which it did not until this PR)

Copy link
Member Author

Choose a reason for hiding this comment

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

quantifier-syntax is an enum type option, even though it looks like an int. If I just add test-assumptions = externs without testing its effects, then it's not testing much is it?

Copy link
Member

Choose a reason for hiding this comment

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

Oh great, you got my point and that's perfect then.

MikaelMayer
MikaelMayer previously approved these changes Sep 5, 2023
@keyboardDrummer keyboardDrummer enabled auto-merge (squash) September 7, 2023 09:24
MikaelMayer
MikaelMayer previously approved these changes Sep 7, 2023
Copy link
Member

@MikaelMayer MikaelMayer left a comment

Choose a reason for hiding this comment

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

Perfect!

MikaelMayer
MikaelMayer previously approved these changes Sep 8, 2023
@@ -9,7 +9,7 @@ Warning: only Dafny project files named dfyconfig.toml are recognised by the Daf
Warning: option 'does-not-exist' that was specified in the project file, is not a valid Dafny option.

Dafny program verifier did not attempt verification
Error: property 'warn-shadowing' is of type 'System.Int64' but should be of type 'System.Boolean'
Error: Could not parse value '3' for option 'warn-shadowing' that has type 'Boolean'
Copy link
Member

Choose a reason for hiding this comment

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

Much better error message.

MikaelMayer
MikaelMayer previously approved these changes Sep 11, 2023
Copy link
Collaborator

@stefan-aws stefan-aws left a comment

Choose a reason for hiding this comment

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

Previously approved

@keyboardDrummer keyboardDrummer merged commit 27eca33 into dafny-lang:master Sep 12, 2023
18 checks passed
@keyboardDrummer keyboardDrummer deleted the supportAllOptionTypesInProjectFile branch September 12, 2023 19:49
keyboardDrummer added a commit to keyboardDrummer/dafny that referenced this pull request Sep 15, 2023
### Changes
- Support all option types in the project file by parsing the project
file options in the same way as the CLI parses them

### Testing
- Added a case that uses an enum option to an existing test

<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
keyboardDrummer added a commit that referenced this pull request Sep 15, 2023
### Changes
- Support all option types in the project file by parsing the project
file options in the same way as the CLI parses them

### Testing
- Added a case that uses an enum option to an existing test

<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
@EkanshdeepGupta
Copy link
Collaborator

EkanshdeepGupta commented Sep 18, 2023

Wow thanks @keyboardDrummer! I just tested it, the feature I implemented can finally be accessed easily. I'm super happy :D

keyboardDrummer added a commit that referenced this pull request Sep 19, 2023
### Changes
- Support all option types in the project file by parsing the project
file options in the same way as the CLI parses them

### Testing
- Added a case that uses an enum option to an existing test

<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
keyboardDrummer added a commit that referenced this pull request Sep 19, 2023
### Changes
- Support all option types in the project file by parsing the project
file options in the same way as the CLI parses them

### Testing
- Added a case that uses an enum option to an existing test

<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
zafer-esen pushed a commit that referenced this pull request Sep 25, 2023
### Changes
- Support all option types in the project file by parsing the project
file options in the same way as the CLI parses them

### Testing
- Added a case that uses an enum option to an existing test

<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
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.

None yet

4 participants