Skip to content

Commit

Permalink
fix RemesPath error with indexers on functions
Browse files Browse the repository at this point in the history
For example, running the query `sum(dict(items(@)).a)` on the JSON `{"a": [1]}`
    now correctly returns `1.0`,
    but RemesPath used to raise an error because it
    assumed that `dict(items(@)).a` had the same type as `dict(items(@))`
  • Loading branch information
molsonkiko committed Sep 1, 2024
1 parent 9740f63 commit db632d4
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 58 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed

1. Fix issue where [random string from regex](/docs/README.md#random-strings-from-regex-added-in-v81) would incorrectly flag some valid regular expressions (e.g. `(?-i)(?:xy{1,2}){,2}`) as having two consecutive quantifiers.
2. Fix issue where RemesPath incorrectly inferred the type of (a [function](/docs/RemesPath.md#functions) `fun` followed by [indexers](/docs/RemesPath.md#indexing-and-selecting-keys)) to be the return type of `fun`. For example, running the query `sum(dict(items(@)).a)` on the JSON `{"a": [1]}` now correctly returns `1.0`, but RemesPath *used to raise an error because it assumed that `dict(items(@)).a` had the same type as `dict(items(@))`*

## [8.1.0] - 2024-08-23

Expand Down
53 changes: 45 additions & 8 deletions JsonToolsNppPlugin/JSONTools/RemesPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
using System.Windows.Forms;
using JSON_Tools.Utils;

namespace JSON_Tools.JSON_Tools
Expand Down Expand Up @@ -103,10 +102,12 @@ public class StarIndexer : Indexer { }
public class Projection : Indexer
{
public Func<JNode, IEnumerable<object>> projFunc;
public Dtype returnType { get; private set; }

public Projection(Func<JNode, IEnumerable<object>> projFunc)
public Projection(Func<JNode, IEnumerable<object>> projFunc, Dtype returnType)
{
this.projFunc = projFunc;
this.returnType = returnType;
}
}

Expand Down Expand Up @@ -1764,6 +1765,19 @@ private Obj_Pos ParseExpr(List<object> toks, int pos, int end, JQueryContext con
// or the next token is the start of a projection (unlike other indexers, projections can operate on scalars too)
var idxrs = new List<IndexerFunc>();
pos = indStartEndPos;
Dtype returnType = lastTok.type;
// the return type of a chain of indexers is a complicated function of the nature and order of the indexers.
// EXAMPLES:
// * @[:].foo and @[1, 2][0]{@.foo, @.bar} must both return arrays, because [:] and [1, 2] both select multiple indices from an array, and the subsequent indexers don't matter
// * @[foo, bar] and @.*[3] and @.g`foo`..* all must return objects, because .* and [foo, bar] and .g`foo` all select multiple keys from an object, and subsequent indexers don't matter
// * @[0].bar[:3] must return an array, because [0].bar returns a single value, and [:3] must return an array
// * @.b[5].c!.d must return an object, because .b[5].c returns a single value, and !.d must return an object
// * @[1]..* must return an array, because [1] returns a single value, and ..* must return an array
// * @{foo: @.quz, bar: sum(@.jfj)} must return an object
// * @{@.baz, @.quz, @.fjjf} must return an array
// * @{@.baz, @.quz, @.fjjf}
// * @->sum(@.foo) must return a number (because sum returns a number)
bool keepChainingReturnType = true;
while (indStart != IndexerStart.NOT_AN_INDEXER)
{
bool isRecursive = IndexerStart.ANY_DOUBLEDOT_TYPE.HasFlag(indStart);
Expand Down Expand Up @@ -1811,6 +1825,21 @@ private Obj_Pos ParseExpr(List<object> toks, int pos, int end, JQueryContext con
else
idxFunc = ApplyMultiIndex(children, isVarnameList, isRecursive);
idxrs.Add(new IndexerFunc(idxFunc, hasOneOption, isProjection, isDict, isRecursive));
if (keepChainingReturnType)
{
if (isRecursive)
{
keepChainingReturnType = false;
returnType = Dtype.ARR;
}
else if (hasOneOption && !isNegated)
returnType = Dtype.UNKNOWN;
else
{
returnType = isVarnameList ? Dtype.OBJ : Dtype.ARR;
keepChainingReturnType = false;
}
}
}
else if (curIdxr is BooleanIndex boodex)
{
Expand All @@ -1820,23 +1849,31 @@ private Obj_Pos ParseExpr(List<object> toks, int pos, int end, JQueryContext con
var idxr = new IndexerFunc(null, hasOneOption, isProjection, isDict, isRecursive);
idxr.idxr = idxr.ApplyBooleanIndex(boodexFun);
idxrs.Add(idxr);
keepChainingReturnType = false;
}
else if (curIdxr is Projection proj)
{
if (isNegated)
throw new RemesPathException("Negated projections are not supported.");
Func<JNode, IEnumerable<object>> projFunc = proj.projFunc;
idxrs.Add(new IndexerFunc(projFunc, hasOneOption, true, false, false));
if (keepChainingReturnType)
returnType = proj.returnType;
}
else
{
// it's a star indexer
if (isNegated)
throw new RemesPathException("Negated star indexers are not supported.");
// it's a star indexer
if (isRecursive)
{
if (keepChainingReturnType)
returnType = Dtype.ARR;
idxrs.Add(new IndexerFunc(RecursivelyFlattenIterable, false, false, false, true));
}
else
idxrs.Add(new IndexerFunc(ApplyStarIndexer, hasOneOption, isProjection, isDict, false));
keepChainingReturnType = false;
}
(indStart, pos) = DetermineIndexerStart(toks, opo.pos, end);
}
Expand All @@ -1850,7 +1887,7 @@ JNode idxFunc(JNode inp)
{
return idxrsFunc(lcur.function(inp));
}
return new Obj_Pos(new CurJson(lcur.type, idxFunc), pos);
return new Obj_Pos(new CurJson(returnType, idxFunc), pos);
}
// if a variable is referenced in the indexers (e.g., "var x = @; range(10)[:]->at(x, @ % len(x))",
// we also need to wait until runtime to evaluate the indexers
Expand All @@ -1860,7 +1897,7 @@ JNode idxFuncVarRef(JNode _)
{
return idxrsFunc(lastTok);
}
return new Obj_Pos(new CurJson(lastTok.type, idxFuncVarRef), pos);
return new Obj_Pos(new CurJson(returnType, idxFuncVarRef), pos);
}
if (lastTok is JObject lastObj)
{
Expand Down Expand Up @@ -2220,7 +2257,7 @@ kv.Value is CurJson cj
);
}
};
return new Obj_Pos(new Projection(projFunc), pos + 1);
return new Obj_Pos(new Projection(projFunc, Dtype.OBJ), pos + 1);
}
else
{
Expand All @@ -2234,7 +2271,7 @@ IEnumerable<object> projFunc(JNode obj)
: node.Copy();
}
};
return new Obj_Pos(new Projection(projFunc), pos + 1);
return new Obj_Pos(new Projection(projFunc, Dtype.ARR), pos + 1);
}
}
if (nd != ',')
Expand Down Expand Up @@ -2271,7 +2308,7 @@ private Obj_Pos ParseMap(List<object> toks, int pos, int end, JQueryContext cont
else
outfunc = x => val.Copy();
IEnumerable<object> iterator(JNode x) { yield return outfunc(x); }
return new Obj_Pos(new Projection(iterator), pos);
return new Obj_Pos(new Projection(iterator, val.type), pos);
}
#endregion
#region EXCEPTION_PRETTIFIER
Expand Down
4 changes: 2 additions & 2 deletions JsonToolsNppPlugin/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@
// Build Number
// Revision
//
[assembly: AssemblyVersion("8.1.0.1")]
[assembly: AssemblyFileVersion("8.1.0.1")]
[assembly: AssemblyVersion("8.1.0.2")]
[assembly: AssemblyFileVersion("8.1.0.2")]
17 changes: 17 additions & 0 deletions JsonToolsNppPlugin/Tests/RemesPathTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,23 @@ public static bool Test()
"\"[\\r\\n 1,\\r\\n {\\r\\n //c1\\r\\n \\\"b\\\": 2,\\r\\n \\\"c\\\": [\\r\\n true,\\r\\n null,\\r\\n -5.5\\r\\n ]\\r\\n },\\r\\n //c2\\r\\n 3.25\\r\\n]\\r\\n\""),
new Query_DesiredResult("s_format(`[1, {\"c\": [true, null, -5.5],//c1\\n \"b\": 2},//c2\\n 3.25]`, p, true, `\\t`, true)",
"\"[\\r\\n\\t1,\\r\\n\\t{\\r\\n\\t\\t//c1\\r\\n\\t\\t\\\"b\\\": 2,\\r\\n\\t\\t\\\"c\\\": [true, null, -5.5]\\r\\n\\t},\\r\\n\\t//c2\\r\\n\\t3.25\\r\\n]\\r\\n\""),
// ====================== correctly infer the return type of an indexer chain ======================
new Query_DesiredResult("sum(dict(@{@{a, j`[[1], [2], [3]]`}}).a[:][0])", "6.0"),
new Query_DesiredResult("sum(dict(@{@{a, j`[[1], [2], [3]]`}}).a[:][0])", "6.0"),
new Query_DesiredResult("add_items(append(@{1}, @{a: 1})[1], b, 2)", "{\"a\": 1, \"b\": 2}"),
new Query_DesiredResult("sum(group_by(@.foo, 1)..*)", "36.0"),
new Query_DesiredResult("sum(dict(@{@{a, 1}}){a: @{1, @{foo: 10}, @{bar: 20.0}}, foo: 6}..[foo, bar])", "36.0"),
new Query_DesiredResult("sum(dict(@{@{a, 1}}){a: @{1, @{foo: 10}, @{bar: 20.0}}, foo: 6}..foo)", "16.0"),
new Query_DesiredResult("sum(dict(@{@{a, 1}}){a: @{1, @{foo: 10}, @{bar: 20.0}}, foo: 6}..g`foo|bar`)", "36.0"),
new Query_DesiredResult("s_join(items(@)->foo, @.bar.b)", "\"a`gfoobah\""),
new Query_DesiredResult("s_join(sum(@.foo[0] + 1)->str(@), j`[\"a\", \"b\", \"c\"]`)", "\"a6.0b6.0c\""),
new Query_DesiredResult("sum(group_by(set(@->j`[1]`)->j`[{\"a\": 1, \"b\": \"foo\"}, {\"a\": 2, \"b\": \"foo\"}, {\"a\": 3, \"b\": \"bar\"}]`, b).foo[:].a)", "3.0"),
new Query_DesiredResult("items(items(@){foo: 1, bar: 2})", "[[\"foo\", 1], [\"bar\", 2]]"),
new Query_DesiredResult("sum(dict(items(@)){@.foo[0][1], @.bar.a})", "1.0"),
new Query_DesiredResult("unique(dict(items(@)).foo[0], true)", "[0, 1, 2]"),
new Query_DesiredResult("items(items(@)[1][1])", "[[\"a\",false],[\"b\",[\"a`g\",\"bah\"]]]"),
new Query_DesiredResult("items(items(@)[1][1][a,b])", "[[\"a\",false],[\"b\",[\"a`g\",\"bah\"]]]"),
new Query_DesiredResult("items(items(@)[1][1].g`[ab]`)", "[[\"a\",false],[\"b\",[\"a`g\",\"bah\"]]]"),
};
int ii = 0;
int testsFailed = 0;
Expand Down
Loading

0 comments on commit db632d4

Please sign in to comment.