Commit 1110c233 authored by Philip Makedonski's avatar Philip Makedonski
Browse files

+ checkNoAnyValueOrNoneInListValues

parent 330cb881

de.ugoe.cs.swe.T3Q/config/t3q-tf160-latest-next-new-features.xml

+3 −0
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ 
      <maximumAllowedNestingDepth>1</maximumAllowedNestingDepth>     <!-- value on defaultProfile of t3q: 0 -->

      <checkNoPermutationKeyword>false</checkNoPermutationKeyword>   <!-- value on defaultProfile of t3q: true -->

      <checkNoAnyTypeKeyword>true</checkNoAnyTypeKeyword>

      <checkNoAnyValueOrNoneInListValues>true</checkNoAnyValueOrNoneInListValues>

      <checkNoModifiedTemplateOfModifiedTemplate>true</checkNoModifiedTemplateOfModifiedTemplate>

      <checkNoAllKeywordInPortDefinitions>true</checkNoAllKeywordInPortDefinitions>

      <checkImportsComeFirst>true</checkImportsComeFirst>
@@ -170,6 +171,7 @@ 
      <maximumAllowedNestingDepth>0</maximumAllowedNestingDepth>

      <checkNoPermutationKeyword>true</checkNoPermutationKeyword>

      <checkNoAnyTypeKeyword>true</checkNoAnyTypeKeyword>

      <checkNoAnyValueOrNoneInListValues>true</checkNoAnyValueOrNoneInListValues>

      <checkNoModifiedTemplateOfModifiedTemplate>true</checkNoModifiedTemplateOfModifiedTemplate>

      <checkNoAllKeywordInPortDefinitions>true</checkNoAllKeywordInPortDefinitions>

      <checkImportsComeFirst>true</checkImportsComeFirst>
@@ -288,6 +290,7 @@ 
      <maximumAllowedNestingDepth>0</maximumAllowedNestingDepth>

      <checkNoPermutationKeyword>false</checkNoPermutationKeyword>

      <checkNoAnyTypeKeyword>false</checkNoAnyTypeKeyword>

      <checkNoAnyValueOrNoneInListValues>true</checkNoAnyValueOrNoneInListValues>

      <checkNoModifiedTemplateOfModifiedTemplate>true</checkNoModifiedTemplateOfModifiedTemplate>

      <checkNoAllKeywordInPortDefinitions>false</checkNoAllKeywordInPortDefinitions>

      <checkImportsComeFirst>false</checkImportsComeFirst>

de.ugoe.cs.swe.T3Q/config/t3q-tf160-latest-next.xml

+3 −0
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ 
      <maximumAllowedNestingDepth>1</maximumAllowedNestingDepth>     <!-- value on defaultProfile of t3q: 0 -->

      <checkNoPermutationKeyword>false</checkNoPermutationKeyword>   <!-- value on defaultProfile of t3q: true -->

      <checkNoAnyTypeKeyword>true</checkNoAnyTypeKeyword>

      <checkNoAnyValueOrNoneInListValues>false</checkNoAnyValueOrNoneInListValues> <!-- new feature, turn off by default -->

      <checkNoModifiedTemplateOfModifiedTemplate>true</checkNoModifiedTemplateOfModifiedTemplate>

      <checkNoAllKeywordInPortDefinitions>true</checkNoAllKeywordInPortDefinitions>

      <checkImportsComeFirst>true</checkImportsComeFirst>
@@ -170,6 +171,7 @@ 
      <maximumAllowedNestingDepth>0</maximumAllowedNestingDepth>

      <checkNoPermutationKeyword>true</checkNoPermutationKeyword>

      <checkNoAnyTypeKeyword>true</checkNoAnyTypeKeyword>

      <checkNoAnyValueOrNoneInListValues>false</checkNoAnyValueOrNoneInListValues> <!-- new feature, turn off by default -->

      <checkNoModifiedTemplateOfModifiedTemplate>true</checkNoModifiedTemplateOfModifiedTemplate>

      <checkNoAllKeywordInPortDefinitions>true</checkNoAllKeywordInPortDefinitions>

      <checkImportsComeFirst>true</checkImportsComeFirst>
@@ -288,6 +290,7 @@ 
      <maximumAllowedNestingDepth>0</maximumAllowedNestingDepth>

      <checkNoPermutationKeyword>false</checkNoPermutationKeyword>

      <checkNoAnyTypeKeyword>false</checkNoAnyTypeKeyword>

      <checkNoAnyValueOrNoneInListValues>false</checkNoAnyValueOrNoneInListValues> <!-- new feature, turn off by default -->

      <checkNoModifiedTemplateOfModifiedTemplate>true</checkNoModifiedTemplateOfModifiedTemplate>

      <checkNoAllKeywordInPortDefinitions>false</checkNoAllKeywordInPortDefinitions>

      <checkImportsComeFirst>false</checkImportsComeFirst>

de.ugoe.cs.swe.T3Q/config/t3q.cfg

+3 −0
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ 
      <maximumAllowedNestingDepth>1</maximumAllowedNestingDepth>     <!-- original value: 0 -->

      <checkNoPermutationKeyword>false</checkNoPermutationKeyword>   <!-- original value: true -->

      <checkNoAnyTypeKeyword>true</checkNoAnyTypeKeyword>

      <checkNoAnyValueOrNoneInListValues>true</checkNoAnyValueOrNoneInListValues>

      <checkNoModifiedTemplateOfModifiedTemplate>true</checkNoModifiedTemplateOfModifiedTemplate>

      <checkNoAllKeywordInPortDefinitions>true</checkNoAllKeywordInPortDefinitions>

      <checkImportsComeFirst>true</checkImportsComeFirst>
@@ -170,6 +171,7 @@ 
      <maximumAllowedNestingDepth>0</maximumAllowedNestingDepth>

      <checkNoPermutationKeyword>true</checkNoPermutationKeyword>

      <checkNoAnyTypeKeyword>true</checkNoAnyTypeKeyword>

      <checkNoAnyValueOrNoneInListValues>true</checkNoAnyValueOrNoneInListValues>

      <checkNoModifiedTemplateOfModifiedTemplate>true</checkNoModifiedTemplateOfModifiedTemplate>

      <checkNoAllKeywordInPortDefinitions>true</checkNoAllKeywordInPortDefinitions>

      <checkImportsComeFirst>true</checkImportsComeFirst>
@@ -286,6 +288,7 @@ 
      <maximumAllowedNestingDepth>0</maximumAllowedNestingDepth>

      <checkNoPermutationKeyword>false</checkNoPermutationKeyword>

      <checkNoAnyTypeKeyword>false</checkNoAnyTypeKeyword>

      <checkNoAnyValueOrNoneInListValues>true</checkNoAnyValueOrNoneInListValues>

      <checkNoModifiedTemplateOfModifiedTemplate>true</checkNoModifiedTemplateOfModifiedTemplate>

      <checkNoAllKeywordInPortDefinitions>false</checkNoAllKeywordInPortDefinitions>

      <checkImportsComeFirst>false</checkImportsComeFirst>

de.ugoe.cs.swe.TTCN3/src/de/ugoe/cs/swe/validation/CodeStyleValidator.xtend

+313 −4
Original line number Diff line number Diff line
@@ -93,6 +93,18 @@ import org.eclipse.emf.ecore.EReference 
import de.ugoe.cs.swe.tTCN3.StructFieldDef

import de.ugoe.cs.swe.tTCN3.OpCall

import de.ugoe.cs.swe.tTCN3.UnionFieldDef

import de.ugoe.cs.swe.tTCN3.RefValue

import de.ugoe.cs.swe.tTCN3.ExtendedFieldReference

import de.ugoe.cs.swe.tTCN3.ReferencedValue

import de.ugoe.cs.swe.tTCN3.Assignment

import de.ugoe.cs.swe.tTCN3.VarInstance

import de.ugoe.cs.swe.tTCN3.ArrayElementExpressionList

import de.ugoe.cs.swe.tTCN3.ArrayExpression

import de.ugoe.cs.swe.tTCN3.SingleExpression

import de.ugoe.cs.swe.tTCN3.RestrictedTemplate

import de.ugoe.cs.swe.tTCN3.TemplateBody

import de.ugoe.cs.swe.tTCN3.SingleTemplateExpression

import de.ugoe.cs.swe.tTCN3.VariableRef



class CheckDefinitionComeFirstParameter {

	public boolean hasOtherDefinitions
@@ -332,6 +344,293 @@ class CodeStyleValidator extends AbstractDeclarativeValidator { 
		}

	}



	def boolean checkValueTemplate(TemplateDef td) {

		if (td.restriction.value !== null || 

			td.restriction.present !== null //TODO: needed?

		) {

			return false

		}

		return true

	}	



	def boolean checkValueTemplate(RestrictedTemplate td) {

		if (td.restriction.value !== null || 

			td.restriction.present !== null //TODO: needed?

		) {

			return false

		}

		return true

	}	



	def boolean checkAnyValueOrNoneUsage(SingleExpression expr) {

		if (expr instanceof Value) {

			if (expr.ref !== null) {

				//var ref = expr.ref.referencedValue

				var ref = expr.ref.head.target

				if (ref instanceof SingleVarInstance) {

					return false

				}

				if (ref instanceof SingleTempVarInstance) {

					var vi = ref.findDesiredParent(VarInstance)

					if (vi.resTemplate !== null) {

						return checkValueTemplate(vi.resTemplate)

					} else {

						return true

					}

				}

				if (ref instanceof BaseTemplate) {

					var td = ref.findDesiredParent(TemplateDef)

					return checkValueTemplate(td)

				}

				if (ref instanceof SingleConstDef) {

					return false

				}

				//TODO: clarify if needed

				if (ref instanceof FormalValuePar) {

					return false

				}

				//TODO: clarify if needed

				if (ref instanceof FormalTemplatePar) {

					if (ref.restriction !== null) {

						return checkValueTemplate(ref.restriction)

					} else {

						return true

					}

				}

			} else if (expr.predef !== null) {

				if (expr.predef instanceof PredefinedValue) {

					if (expr.predef.omit !== null) {

						return true

					}

				}

			}

		} else if (expr instanceof OpCall) {

			if (expr.function !== null) {

				//TODO: oversimplified?

				if (expr.function.pre !== null) {

					return false

				}

				if (expr.function.ref instanceof FunctionDef) {

					var returnType = (expr.function.ref as FunctionDef).returnType

					if (returnType.template !== null) {

						//TODO: refine?

						return true

					} else if (returnType.restricted !== null) {

						return checkValueTemplate(returnType.restricted)

					}

				}

				//DONE: handle parameterised template references?		

				if (expr.function.ref instanceof BaseTemplate) {

					var td = expr.function.ref.findDesiredParent(TemplateDef)

					return checkValueTemplate(td)

				}

				//TODO: others?

			}

			if (expr.templateOps !== null) {

				return false

//				if (expr.templateOps instanceof ValueofOp) {

//					return false

//				}

			}

		}

		return false

	}



	@Check

	def checkNoAnyValueOrNoneInListValues(ExtendedFieldReference r) {

		if (!activeProfile.checkNoAnyValueOrNoneInListValues)

			return;



		if (r.eContainer instanceof Type) {

			return //not applicable

		}

		

		if (r.nextSibling !== null) {

			return //not applicable

		}

		//TODO: update configuration version

		//TODO: add documentation and example

		

		//TODO: other usage cases?		

		if (r.array !== null) {

			//TODO: check if guaranteed, resolve and include in warning

			//TODO: check if needed for confirmation of type

			

			//TODO: export to reusable debugging dump

			//val debugNode = NodeModelUtils.findActualNodeFor(r.eContainer)

			//println(debugNode.startLine+" : "+debugNode.text)

			var v = (r.eContainer as ReferencedValue).referencedValue.name

			

			//DONE: distinguish between cases based on the assignment

			//negative distinction => exit scenarios

			var assignment = r.findDesiredParent(Assignment)

			//body

			if (assignment.body !== null) {

				//skip if within right-hand-side in assignment

				//naive: top level only

//				var body = r.findDesiredParent(TemplateBody)

//				if (assignment.body === body) {

//					return					

//				}

				if (! (r.eContainer.eContainer instanceof VariableRef)) {

					return

				}

				

				var expr = assignment.body.simple.expr



				//TODO: this should be improved

				if (expr instanceof ArrayExpression) {

					checkArrayExpression(expr as ArrayExpression, v)

				}

				var present = checkAnyValueOrNoneUsage(expr)

				

				if (assignment.body.simple.spec !== null) {

					var tExpr = assignment.body.simple.spec.expr

					if (tExpr instanceof SingleTemplateExpression) {

						if (tExpr.symbol !== null) { 

							if (tExpr.symbol.anyornone !== null) {

								present = true

							}

						}

					}

				}

				if (!present) {

					return

				}

			}

			

			//expression

			if (assignment.expression !== null) {

				//handle?

			}

			

			//extra?

			//TODO: handle?

			

			statistics.incrementCountStyle

			val INode node = NodeModelUtils.getNode(r)

			val message = "\"AnyValueOrNone\" used as a list value in \""+v+"\"!"

			warning(

				message,

				r.eContainer,

				null,

				MessageClass.STYLE.toString,

				node.startLine.toString,

				node.endLine.toString,

				"6.15, " + MiscTools.getMethodName()

			);

		}

	}



	def checkArrayExpression(ArrayExpression expr, String target) {

		//with minimal adjustments -> extract and reuse?

		for (v : expr.list.expr) {

			if (v instanceof Value) {

				val present = checkAnyValueOrNoneUsage(v)

				if (present) {

					statistics.incrementCountStyle

					val INode node = NodeModelUtils.getNode(v)

					//DONE: clarify if only the parameter declaration is to be checked -> no, usage critical

					//DONE: clarify error messages if only the parameter declaration is to be used (is vs can?) -> is focus on usage

					//TODO: shift to usage-based, update warning

					//TODO: distinguish between list / set or write both?

					//TODO: check also usage of other templates, not just parameters?

					val message = "\"AnyValueOrNone\" is used as a list value in \""+target+"\"!"

					warning(

						message,

						v,

						null,

						MessageClass.STYLE.toString,

						node.startLine.toString,

						node.endLine.toString,

						"6.15, " + MiscTools.getMethodName()

					);

				}

				//other cases? const?

			}

		}

	}



	@Check

	def checkNoAnyValueOrNoneInListValues(SingleTempVarInstance vi) {

		if (!activeProfile.checkNoAnyValueOrNoneInListValues)

			return;

		val type = vi.findDesiredParent(VarInstance).type

		

		//TODO: duplicated from below

		if (type.pre !== null) {

			return

		}

		if (type.ref !== null) {

			var refType = type.ref.head

			if (!(refType instanceof RecordOfDefNamed || refType instanceof SetOfDefNamed)) {

				return

			}

			var expr = vi.template.simple.expr

			//check if ArrayExpression, skip otherwise

			if (! (expr instanceof ArrayExpression)){

				return

			}

			checkArrayExpression(expr as ArrayExpression, vi.name)

		}

	}



	@Check

	def checkNoAnyValueOrNoneInListValues(TemplateDef template) {

		if (!activeProfile.checkNoAnyValueOrNoneInListValues)

			return;

		val type = template.base.type

		if (type.pre !== null) {

			return

		}

		//TODO: change logic to focus on use -> merge above?

		if (template.base.type.ref !== null) {

			var refType = template.base.type.ref.head

			if (!(refType instanceof RecordOfDefNamed || refType instanceof SetOfDefNamed)) {

				return

			}

			

			var expr = template.body.simple.expr

			//check if ArrayExpression, skip otherwise -> handle?

			if (! (expr instanceof ArrayExpression)){

				return

			}

			

			checkArrayExpression(expr as ArrayExpression, template.base.name) 

			

			//old implementation based on parameter definitions -> remove

//			for (p : template.base.parList.params) {

//				if (p.template !== null) { //is template and

//					if (p.template.restriction === null //and has no restriction

//						|| //or

//						(p.template.restriction !== null && //has restriction 

//						 p.template.restriction.restriction.omit !== null) //which is omit

//					) {

//						var v = template.base.name

//						statistics.incrementCountStyle

//						val INode node = NodeModelUtils.getNode(p.template)

//						//DONE: clarify if only the parameter declaration is to be checked -> no, usage critical

//						//DONE: clarify error messages if only the parameter declaration is to be used (is vs can?) -> is focus on usage

//						//TODO: shift to usage-based, update warning

//						//TODO: distinguish between list / set or write both?

//						//TODO: check also usage of other templates, not just parameters?

//						val message = "\"AnyValueOrNone\" is used as a list value in \""+v+"\"!"

//						warning(

//							message,

//							p.template,

//							null,

//							MessageClass.STYLE.toString,

//							node.startLine.toString,

//							node.endLine.toString,

//							"6.15, " + MiscTools.getMethodName()

//						);

//					}

//				}

//			}

		}

	}

		



	@Check

	def checkNoModifiedTemplateOfModifiedTemplate(TemplateDef template) {

		if (!activeProfile.checkNoModifiedTemplateOfModifiedTemplate)
@@ -891,6 +1190,7 @@ class CodeStyleValidator extends AbstractDeclarativeValidator { 
		//TODO: overly specific.. 

		//TODO: overly complicated -> simplify and reduce/invert conditions

		var target = ""

		var prefix = ""

		val expr = templateOp.template.template.simple.expr

//		.ref.head

		if (expr instanceof Value) {
@@ -914,8 +1214,15 @@ class CodeStyleValidator extends AbstractDeclarativeValidator { 
						return

					}

				}

				

				//TODO: refer to base

				if (expr.ref.head.target instanceof RefValue) {

					target = (expr.ref.head.target as RefValue).name

				} else {

					//TODO: handle further?

					target = expr.ref.referencedValue.name

				}

				

//				target = expr.ref.head.target.name

				//TODO: other conditions?

				

			} else {
@@ -932,8 +1239,10 @@ class CodeStyleValidator extends AbstractDeclarativeValidator { 
					if (returnType.restricted !== null) { 

						return	

					}

					//TODO: add field or revert to template above?

					//TODO: add field or revert to template above? -> use function ref, in case with prefix

					//TODO: update documentation?

					target = expr.function.ref.name

					prefix = "returned by "

				} else if (expr.function.ref instanceof BaseTemplate) {

					return

				}
@@ -944,8 +1253,8 @@ class CodeStyleValidator extends AbstractDeclarativeValidator { 
			return

		}

		statistics.incrementCountStyle



		val message = "ValueOf operation is used on a value \""+target+"\"!"

		//TODO: target shall be the base value (already the case for functionrefs, may need to be tuned, e.g. returned by?)

		val message = "ValueOf operation is used on a value "+prefix+"\""+target+"\"!"

		val INode node = NodeModelUtils.getNode(templateOp)

		warning(

			message,

de.ugoe.cs.swe.TTCN3Configuration/src/de/ugoe/cs/swe/TTCN3Configuration/QualityCheckProfile.java

+9 −0
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ public class QualityCheckProfile extends ConfigurationProfile { 
	private int maximumAllowedNestingDepth = 0;

	private boolean checkNoPermutationKeyword = true;

	private boolean checkNoAnyTypeKeyword = true;

	private boolean checkNoAnyValueOrNoneInListValues = true;

	private boolean checkNoModifiedTemplateOfModifiedTemplate = true;



	private boolean checkNoAllKeywordInPortDefinitions = true;
@@ -620,6 +621,14 @@ public class QualityCheckProfile extends ConfigurationProfile { 
		this.checkNoValueOfForValues = checkNoValueOfForValues;

	}



	public boolean isCheckNoAnyValueOrNoneInListValues() {

		return checkNoAnyValueOrNoneInListValues;

	}



	public void setCheckNoAnyValueOrNoneInListValues(boolean checkNoAnyValueOrNoneInListValues) {

		this.checkNoAnyValueOrNoneInListValues = checkNoAnyValueOrNoneInListValues;

	}



	



}