Skip to content

Conversation

@leppie
Copy link
Contributor

@leppie leppie commented Nov 13, 2015

Fixes #6109.

Please advise if tests are needed and where they meant to be.

/cc @gafter

@gafter gafter changed the title Fixes #6109 Correct scanning for literals such as 1.0e-999999999999999999999 (should convert to zero) Nov 13, 2015
Copy link
Member

Choose a reason for hiding this comment

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

Please do not use exception handling.

@gafter
Copy link
Member

gafter commented Nov 13, 2015

I recommend adding tests in https://github.com/dotnet/roslyn/blob/master/src/Compilers/Core/CodeAnalysisTest/RealParserTests.cs

There are currently no "negative" tests for overflow situations, but this bug fix is for a "positive" situation where the number should be accepted but is rejected.

@leppie
Copy link
Contributor Author

leppie commented Nov 14, 2015

This is turning into an endless battle... 😞

Reporting CS0594 for invalid floating point literals seems incorrect. These should be rejected by the tokenizer/lexer already. Examples:0e, 5e-, .1e+.

@leppie
Copy link
Contributor Author

leppie commented Nov 14, 2015

Too much spaghetti logic involved to fix this now, will maybe try again at a later stage.

@leppie leppie closed this Nov 14, 2015
@gafter
Copy link
Member

gafter commented Nov 14, 2015

@leppie this is part of the scanner.

@leppie
Copy link
Contributor Author

leppie commented Nov 14, 2015

@gafter The scanner is allowing nonsense to pass (https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/Parser/Lexer.cs#L1032) and there is no error that I can see that allows for indicating an invalid real literal except CS0594 which is also nonsense for these cases. If there was at least some error like Invalid Number like for integers, that could be applied, but this does not seem the case. I also dont think I am in a position to create a new error message for this case.

@davkean davkean added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Dec 16, 2015
github-actions bot pushed a commit that referenced this pull request Apr 1, 2025
* Add analyzer and fixer for CA1865

This analyzer detects when either `Set.Add` or `Set.Remove` is guarded
by `Set.Contains`. A fix is only suggested when the conditional body
has a single statement that is either one of the former two methods.

* Add CA1865 to Microsoft.CodeAnalysis.NetAnalyzers.md

* Add CA1865 to RulesMissingDocumentation.md

* Update DoNotGuardSetAddOrRemoveByContainsDescription

* Add ExportCodeFixProviderAttribute

* Remove call to First with call to indexer

* Change equivalenceKey of CodeAction

* Check for interface implementation instead of signature match

* Remove call to First with call to indexer

* Add additional tests

* Fix ifs with additional statements

* Fix single line ifs in VB

* Add fixer case for single line ifs in VB

* CA1865: Add tests for other set types

* CA1865: Check descendants instead of switch

* CA1865: Do not fire when arguments are different

* CA1865: Change fixer title

* CA1865: Update resources

* WIP: Try to handle IImmutableSet Add and Remove

* Apply suggestions from code review

* CA1865: Use original definitions to filter methods

* Check parameter length before using them

* Only check child operations instead of descendants

This ensures that nested Add or Remove calls are not flagged.

* Fix diagnostic message

* Also flag when interface types are used

* Use helper class for required symbols

* Only compare first argument value

This is sufficient as each method involved only has one argument.

* Make symbol display format static

* Update DoNotGuardSetAddOrRemoveByContainsTitle

* Change condition to pattern matching

* Add tests for ternary operators

* Use raw strings and inline class and usings

* Support Add or Remove in else block

* Support ternary operator (diagnostic only)

* Move from CA1865 to CA1868

---------

Co-authored-by: Buyaa Namnan <buyankhishig.namnan@microsoft.com>
Co-authored-by: Sam Harwell <sam@tunnelvisionlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-already-signed Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants