Merge branch 'master' into manifest-features

__tests__/f1.test.js

@@ -542,6 +542,44 @@ describe('Rules', () => {
 			expect(result).not.toContainMessage(error);
 		});
 
+		it('should not error for fields with multiple references in html {% %}', () => {
+			let result = rule(parse(`model: my_model {
+				view: foo {
+					sql_table_name: foo ;;
+					dimension: bar { 
+						html: {% if bat._value == "usd" or bat._value == "cad" %}
+							<p>\${{rendered_value}}</p>
+							{% elsif bat._value == "eur" %}
+							<p>€{{rendered_value}}</p>
+							{% else %}
+							<p>{{rendered_value}}</p>
+							{% endif %} ;;
+						}
+				}
+			}`));
+			expect(result).toContainMessage(summary(1, 0, 0));
+			expect(result).toContainMessage({...F1});
+		});
+
+		it('should error for 3-part cross-view special-suffix with multiple references in html {% %}', () => {
+			let result = rule(parse(`model: my_model {
+				view: foo {
+					sql_table_name: foo ;;
+					dimension: bar { 
+						html: {% if bat._value == "usd" or baz.bat._value == "cad" %}
+							<p>\${{rendered_value}}</p>
+							{% elsif bat._value == "eur" %}
+							<p>€{{rendered_value}}</p>
+							{% else %}
+							<p>{{rendered_value}}</p>
+							{% endif %} ;;
+						}
+				}
+			}`));
+			expect(result).toContainMessage(summary(1, 0, 1));
+			expect(result).toContainMessage({...F1, ...error});
+		});
+
 		// TODO: Expose assembleModels from parser to test extensions here without full parseFiles & dummy project test case
 		// it('should not error for a view with extension:required', () => {
 		// 	let result = rule(parse(`model: my_model {

docs/release-notes/v3.md

@@ -76,3 +76,4 @@ Patches:
 	- v3.1.2
 		- Fix license metadata to consistently reflect MIT license
 		- Update vulnerable dependencies (both in root package and tools/rule-sandbox)
+	- v3.1.3 - Fix rule F1 logic to check all references when there are multiple in a field ([Issue 167](https://github.com/looker-open-source/look-at-me-sideways/issues/167) [PR 168](https://github.com/looker-open-source/look-at-me-sideways/pull/168))

npm-shrinkwrap.dev.json

@@ -1,12 +1,12 @@
 {
   "name": "@looker/look-at-me-sideways",
-  "version": "3.1.2",
+  "version": "3.1.3",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "@looker/look-at-me-sideways",
-      "version": "3.1.2",
+      "version": "3.1.3",
       "license": "MIT",
       "dependencies": {
         "dot": "^1.1.3",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@looker/look-at-me-sideways",
-  "version": "3.1.2",
+  "version": "3.1.3",
   "description": "A judgmental little LookML linter >_>",
   "main": "index.js",
   "directories": {

rules/f1.js

@@ -43,38 +43,57 @@ function ruleFn(match, path, project) {
 		if (!referenceContainer || !referenceContainer.replace) {
 			continue;
 		}
-		// TODO: Currently only checking the first reference in each block/container, should loop through them all
-		let regexMatch = referenceContainer
-			.replace(/\b\d+\.\d+\b/g, '') // Remove decimals
-			.match(/(\$\{|\{\{|\{%)\s*(([^.{}]+)(\.[^.{}]+)+)\s*(%\}|\})/);
-		let parts = ((regexMatch || [])[2] || '').split('.').map((p)=>p.trim()).filter(Boolean);
-		if (!parts.length) {
-			continue;
-		}
-		// Don't treat references to TABLE or to own default alias as cross-view
-		if (parts[0] === 'TABLE' || parts[0] === view.$name) {
-			parts.shift();
+
+		referenceContainer = referenceContainer.replace(/\b\d+\.\d+\b/g, ''); // Remove decimals
+
+		let regexPattern = /(\$\{|\{\{|\{%)\s*(([^.{}]+)(\.[^.{}]+)+)\s*(%\}|\})/g;
+
+		let matches = [];
+		let match;
+		while ((match = regexPattern.exec(referenceContainer)) !== null) {
+			matches.push(...match[2].trim().split(' ').filter((str) => str.includes('.')).filter(Boolean));
 		}
-		// Don't treat references to special properties as cross-view
-		// Note: view._in_query,_is_filtered,_is_selected should not be allowed in fields
-		if ([
-			'SQL_TABLE_NAME',
-			'_sql',
-			'_value',
-			'_name',
-			'_filters',
-			'_parameter_value',
-			'_rendered_value',
-			'_label',
-			'_link',
-		].includes(parts[parts.length - 1])
-		) {
-			parts.pop();
+
+		let fieldPaths = [...new Set(matches)]; // remove duplicates
+
+		if (!fieldPaths.length) {
+			continue;
 		}
-		if (parts.length > 1) {
+
+		// find all of the field paths that uses cross view references
+		let crossViewFieldPaths = fieldPaths.filter((fieldPath) => {
+			let parts = fieldPath.split('.');
+
+			// Don't treat references to TABLE or to own default alias as cross-view
+			if (parts[0] === 'TABLE' || parts[0] === view.$name) {
+				return false;
+			}
+
+			// Don't treat references to special properties as cross-view
+			// Note: view._in_query,_is_filtered,_is_selected should not be allowed in fields
+			if (parts.length == 2 && [ // only matches the fields that have two parts
+				'SQL_TABLE_NAME',
+				'_sql',
+				'_value',
+				'_name',
+				'_filters',
+				'_parameter_value',
+				'_rendered_value',
+				'_label',
+				'_link',
+			].includes(parts[parts.length - 1])
+			) {
+				return false;
+			}
+			return true;
+		});
+
+		if (crossViewFieldPaths.length > 0) {
+			// only report the first cross-view reference error
+			let parts = crossViewFieldPaths[0].split('.');
 			messages.push({
 				level: 'error',
-				description: `${field.$name} references another view, ${parts[0]},  via ${parts.join('.')}`,
+				description: `${field.$name} references another view, ${parts[0]},  via ${crossViewFieldPaths[0]}`,
 			});
 		}
 	}