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

Make int and float parsing more strict #191

Merged
merged 2 commits into from
Apr 20, 2018
Merged

Make int and float parsing more strict #191

merged 2 commits into from
Apr 20, 2018

Conversation

pfarrel
Copy link
Contributor

@pfarrel pfarrel commented Apr 18, 2018

Fix for #184 reported by @anthmatic.
#178 unintentionally made int and float parsing a lot more lax. This change restores some of the original strictness to match behaviour with jinja , while keeping support for our locale specific changes. It does so by:

  • Checking that all of the input string was consumed by the parse operation, to ensure that trailing symbols or characters cause a parse to fail.
  • Parsing integers first as a float, then converting to an integer, so that the above change doesn't prevent decimal expressions from parsing as integers.

@pfarrel pfarrel requested review from boulter and mattcoley April 18, 2018 16:08
@codecov-io
Copy link

codecov-io commented Apr 18, 2018

Codecov Report

Merging #191 into master will increase coverage by 0.18%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #191      +/-   ##
============================================
+ Coverage     71.18%   71.37%   +0.18%     
- Complexity     1344     1354      +10     
============================================
  Files           215      215              
  Lines          4269     4297      +28     
  Branches        680      685       +5     
============================================
+ Hits           3039     3067      +28     
+ Misses          988      987       -1     
- Partials        242      243       +1
Impacted Files Coverage Δ Complexity Δ
...java/com/hubspot/jinjava/lib/filter/IntFilter.java 100% <100%> (ø) 8 <0> (+2) ⬆️
...va/com/hubspot/jinjava/lib/filter/FloatFilter.java 90.9% <100%> (+2.02%) 8 <0> (+2) ⬆️
...ava/com/hubspot/jinjava/el/ExpressionResolver.java 92.3% <0%> (+3.67%) 14% <0%> (+6%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b5c62e...1319105. Read the comment docs.

@mattcoley
Copy link
Collaborator

Can we add a test to make sure parsing values such as 1.100000 still works as expected? Should work now but just want to cover all bases for future changes.

@pfarrel pfarrel merged commit 05f41e6 into master Apr 20, 2018
@pfarrel pfarrel deleted the int-float-parsing branch April 20, 2018 08:29
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