widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/codecheck-braces into lp:widelands.

Commit message:
Added a codecheck rule to require a space before opening braces.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/codecheck-braces/+merge/286466

There was a bunch of "if (something){" in the code that bothered me.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/codecheck-braces into lp:widelands.

=== added file 'cmake/codecheck/rules/missing_space_before_opening_brace'
--- cmake/codecheck/rules/missing_space_before_opening_brace	1970-01-01 00:00:00 +0000
+++ cmake/codecheck/rules/missing_space_before_opening_brace	2016-02-18 08:57:58 +0000
@@ -0,0 +1,29 @@
+#!/usr/bin/python
+
+
+"""
+Opening braces need to be at the start of a line, or be preceded by a space.
+"""
+
+error_msg = "A space is mandatory before '{' unless it is part of an expression or at the start of a line."
+
+strip_comments_and_strings = True
+regexp = r"""[^\w\s\t\[({<>]{"""
+
+
+forbidden = [
+    "if (something){",
+]
+
+allowed = [
+    "if (something) {",
+    "{",
+    "\t{",
+    " {",
+    "container.at({a, tuple})",
+    "container[{a, tuple}]",
+    "<{a, tuple}>",
+    "InitDataType{a, tuple}",
+    "{{a, nested}, tuple}",
+    "std::map<std::string, std::string>{a, tuple}",
+]

=== modified file 'src/ai/ai_help_structs.h'
--- src/ai/ai_help_structs.h	2016-01-20 20:12:00 +0000
+++ src/ai/ai_help_structs.h	2016-02-18 08:57:58 +0000
@@ -589,7 +589,7 @@
 
 	SchedulerTask
 		(const uint32_t time, const Widelands::SchedulerTaskId t, const uint8_t p, const char* d):
-		due_time(time), id(t), priority(p), descr(d){}
+		due_time(time), id(t), priority(p), descr(d) {}
 
 };
 

=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc	2016-02-13 12:15:29 +0000
+++ src/ai/defaultai.cc	2016-02-18 08:57:58 +0000
@@ -1502,7 +1502,7 @@
 	bool needs_boost_economy = false;
 	if (highest_nonmil_prio_ > 10
 		&& has_enough_space
-		&& virtual_mines >= 5){
+		&& virtual_mines >= 5) {
 			needs_boost_economy = true;
 		}
 
@@ -1758,7 +1758,7 @@
 
 		} else if (bo.type == BuildingObserver::MILITARYSITE) {
 			bo.new_building_ = check_building_necessity(bo.desc->get_size(), gametime);
-		} else if  (bo.type == BuildingObserver::TRAININGSITE){
+		} else if  (bo.type == BuildingObserver::TRAININGSITE) {
 			bo.new_building_ = check_building_necessity(bo, PerfEvaluation::kForConstruction, gametime);
 		} else if (bo.aimode_limit_status() != AiModeBuildings::kAnotherAllowed) {
 			bo.new_building_ = BuildingNecessity::kNotNeeded;

=== modified file 'src/economy/fleet.cc'
--- src/economy/fleet.cc	2016-02-07 06:10:47 +0000
+++ src/economy/fleet.cc	2016-02-18 08:57:58 +0000
@@ -337,14 +337,14 @@
 	return true;
 }
 
-uint32_t Fleet::count_ships(){
+uint32_t Fleet::count_ships() {
 	return ships_.size();
 }
 
-uint32_t Fleet::count_ships_heading_here(EditorGameBase & egbase, PortDock * port){
+uint32_t Fleet::count_ships_heading_here(EditorGameBase & egbase, PortDock * port) {
 	uint32_t ships_on_way = 0;
-	for (uint16_t s = 0; s < ships_.size(); s += 1){
-		if (ships_[s]->get_destination(egbase) == port){
+	for (uint16_t s = 0; s < ships_.size(); s += 1) {
+		if (ships_[s]->get_destination(egbase) == port) {
 			ships_on_way += 1;
 		}
 	}
@@ -352,10 +352,10 @@
 	return ships_on_way;
 }
 
-uint32_t Fleet::count_ports(){
+uint32_t Fleet::count_ports() {
 	return ports_.size();
 }
-bool Fleet::get_act_pending(){
+bool Fleet::get_act_pending() {
 	return act_pending_;
 }
 
@@ -612,7 +612,7 @@
 {
 	for (PortDock * temp_port : ports_) {
 		for (Coords tmp_coords :  temp_port->get_positions(egbase)) {
-			if (tmp_coords == field_coords){
+			if (tmp_coords == field_coords) {
 				return temp_port;
 			}
 		}
@@ -636,7 +636,7 @@
  */
 void Fleet::update(EditorGameBase & egbase)
 {
-	if (act_pending_){
+	if (act_pending_) {
 		return;
 	}
 
@@ -692,7 +692,7 @@
 	// first we go over ships - idle ones (=without destination)
 	// then over wares on these ships and create first ship-port
 	// pairs with score
-	for (uint16_t s = 0; s < ships_.size(); s += 1){
+	for (uint16_t s = 0; s < ships_.size(); s += 1) {
 		if (ships_[s]->get_destination(game)) {
 			continue;
 		}
@@ -700,13 +700,13 @@
 			continue; // in expedition obviously
 		}
 
-		for (uint16_t i = 0; i < ships_[s]->get_nritems(); i += 1){
+		for (uint16_t i = 0; i < ships_[s]->get_nritems(); i += 1) {
 			PortDock * dst = ships_[s]->items_[i].get_destination(game);
 			if (!dst) {
 				// if wares without destination on ship without destination
 				// such ship can be send to any port, and should be sent
 				// to some port, so we add 1 point to score for each port
-				for (uint16_t p = 0; p < ports_.size(); p += 1){
+				for (uint16_t p = 0; p < ports_.size(); p += 1) {
 					mapping.first = s;
 					mapping.second = p;
 					scores[mapping] += 1;
@@ -715,15 +715,15 @@
 			}
 
 			bool destination_found = false; //just a functional check
-			for (uint16_t p = 0; p < ports_.size(); p += 1){
-				if (ports_[p] ==  ships_[s]->items_[i].get_destination(game)){
+			for (uint16_t p = 0; p < ports_.size(); p += 1) {
+				if (ports_[p] ==  ships_[s]->items_[i].get_destination(game)) {
 					mapping.first = s;
 					mapping.second = p;
 					scores[mapping] += (i == 0)?8:1;
 					destination_found = true;
 				}
 			}
-			if (!destination_found){
+			if (!destination_found) {
 				// Perhaps the throw here is too strong
 				// we can still remove it before stable release if it proves too much
 				// during my testing this situation never happened
@@ -737,9 +737,9 @@
 
 	// now opposite aproach - we go over ports to find out those that have wares
 	// waiting for ship then find candidate ships to satisfy the requests
-	for (uint16_t p = 0; p < ports_.size(); p += 1){
+	for (uint16_t p = 0; p < ports_.size(); p += 1) {
 		PortDock & pd = *ports_[p];
-		if (!pd.get_need_ship()){
+		if (!pd.get_need_ship()) {
 			continue;
 		}
 
@@ -753,7 +753,7 @@
 
 		// scoring and entering the pair into scores (or increasing existing
 		// score if the pair is already there)
-		for (uint16_t s = 0; s < ships_.size(); s += 1){
+		for (uint16_t s = 0; s < ships_.size(); s += 1) {
 
 			if (ships_[s]->get_destination(game)) {
 				continue; // already has destination
@@ -824,18 +824,18 @@
 	uint16_t best_score;
 
 	// after sending a ship we will remove one or more items from scores
-	while (!scores.empty()){
+	while (!scores.empty()) {
 		best_score = 0;
 
 		// searching for combination with highest score
 		for (std::pair<std::pair<uint16_t, uint16_t>, uint16_t> combination : scores) {
-			if (combination.second > best_score){
+			if (combination.second > best_score) {
 				best_score = combination.second;
 				best_ship = combination.first.first;
 				best_port = combination.first.second;
 			}
 		}
-		if (best_score == 0){
+		if (best_score == 0) {
 			// this is check of correctnes of this algorithm, this should not happen
 			throw wexception("Fleet::act(): No port-destination pair selected or its score is zero");
 		}
@@ -853,7 +853,7 @@
 
 		// pruning the scores table
 		// the ship that was just sent somewhere cannot be send elsewhere :)
-		for (auto it = scores.cbegin(); it != scores.cend();){
+		for (auto it = scores.cbegin(); it != scores.cend();) {
 
 			// decreasing score for target port as there was a ship just sent there
 			if (it->first.second == best_port) {

=== modified file 'src/economy/portdock.cc'
--- src/economy/portdock.cc	2016-02-09 16:29:48 +0000
+++ src/economy/portdock.cc	2016-02-18 08:57:58 +0000
@@ -364,7 +364,7 @@
 			waiting_.pop_back();
 		}
 
-		if (waiting_.empty()){
+		if (waiting_.empty()) {
 			set_need_ship(game, false);
 		}
 	}

=== modified file 'src/logic/map.cc'
--- src/logic/map.cc	2016-02-14 14:09:29 +0000
+++ src/logic/map.cc	2016-02-18 08:57:58 +0000
@@ -1833,21 +1833,21 @@
 
 	// remove invalid resources if necessary
 	// check vertex to which the triangle belongs
-	if (!is_resource_valid(world, c, c.field->get_resources())){
+	if (!is_resource_valid(world, c, c.field->get_resources())) {
 		clear_resources(c);
 	}
 
 	// always check south-east vertex
 	Widelands::FCoords f_se(c, c.field);
 	get_neighbour(f_se, Widelands::WALK_SE, &f_se);
-	if (!is_resource_valid(world, f_se, f_se.field->get_resources())){
+	if (!is_resource_valid(world, f_se, f_se.field->get_resources())) {
 		clear_resources(f_se);
 	}
 
 	// check south-west vertex if d-Triangle is changed, check east vertex if r-Triangle is changed
 	Widelands::FCoords f_sw_e(c, c.field);
 	get_neighbour(f_sw_e, c.t == TCoords<FCoords>::D ? Widelands::WALK_SW : Widelands::WALK_E, &f_sw_e);
-	if (!is_resource_valid(world, f_sw_e, f_sw_e.field->get_resources())){
+	if (!is_resource_valid(world, f_sw_e, f_sw_e.field->get_resources())) {
 		clear_resources(f_sw_e);
 	}
 

=== modified file 'src/logic/map_objects/immovable.cc'
--- src/logic/map_objects/immovable.cc	2016-02-10 19:50:13 +0000
+++ src/logic/map_objects/immovable.cc	2016-02-18 08:57:58 +0000
@@ -118,7 +118,7 @@
 	FCoords const f = map.get_fcoords(c);
 
 	// this is to help to debug failing assertion below (see bug 1542238)
-	if (f.field->immovable != this){
+	if (f.field->immovable != this) {
 		log(" Internal error: Immovable at %3dx%3d does not match: is %s but %s was expected.\n",
 		c.x,
 		c.y,

=== modified file 'src/logic/map_objects/tribes/production_program.cc'
--- src/logic/map_objects/tribes/production_program.cc	2016-02-15 22:31:59 +0000
+++ src/logic/map_objects/tribes/production_program.cc	2016-02-18 08:57:58 +0000
@@ -1638,7 +1638,7 @@
 			std::vector<ImmovableFound> found_immovables;
 			const uint32_t imm_count =
 				map.find_immovables(Area<FCoords>(map.get_fcoords(coords), 2), &found_immovables);
-			if (best_score > imm_count){
+			if (best_score > imm_count) {
 				best_score = imm_count;
 				best_coords = coords;
 			}

=== modified file 'src/logic/map_objects/tribes/productionsite.cc'
--- src/logic/map_objects/tribes/productionsite.cc	2016-02-15 22:31:59 +0000
+++ src/logic/map_objects/tribes/productionsite.cc	2016-02-18 08:57:58 +0000
@@ -301,7 +301,7 @@
 			for (const auto& wp : bld->working_positions()) {
 
 				// If worker for this position is buildable, just skip him
-				if (owner().tribe().get_worker_descr(wp.first)->is_buildable()){
+				if (owner().tribe().get_worker_descr(wp.first)->is_buildable()) {
 					continue;
 				}
 

=== modified file 'src/logic/map_objects/tribes/ship.cc'
--- src/logic/map_objects/tribes/ship.cc	2016-02-11 06:50:56 +0000
+++ src/logic/map_objects/tribes/ship.cc	2016-02-18 08:57:58 +0000
@@ -745,7 +745,7 @@
 /**
  * Find a path to the dock @p pd, returns its length, and the path optionally.
  */
-uint32_t Ship::calculate_sea_route(Game& game, PortDock& pd, Path* finalpath){
+uint32_t Ship::calculate_sea_route(Game& game, PortDock& pd, Path* finalpath) {
 	Map& map = game.map();
 	StepEvalAStar se(pd.get_warehouse()->get_position());
 	se.m_swim = true;
@@ -760,7 +760,7 @@
 	FCoords cur;
 	while (astar.step(cur, cost)) {
 		if (cur.field->get_immovable() == &pd) {
-			if (finalpath){
+			if (finalpath) {
 				astar.pathto(cur, *finalpath);
 				return finalpath->get_nsteps();
 			} else {

=== modified file 'src/logic/map_objects/tribes/warehouse.cc'
--- src/logic/map_objects/tribes/warehouse.cc	2016-02-15 22:31:59 +0000
+++ src/logic/map_objects/tribes/warehouse.cc	2016-02-18 08:57:58 +0000
@@ -1477,9 +1477,9 @@
 {
 	Building::log_general_info(egbase);
 
-	if (descr().get_isport()){
+	if (descr().get_isport()) {
 		PortDock* pd_tmp = portdock_;
-		if (pd_tmp){
+		if (pd_tmp) {
 			molog("Port dock: %u\n", pd_tmp->serial());
 			molog("port needs ship: %s\n", (pd_tmp->get_need_ship())?"true":"false");
 			molog("wares and workers waiting: %u\n", pd_tmp->count_waiting());

=== modified file 'src/logic/player.cc'
--- src/logic/player.cc	2016-02-14 14:09:29 +0000
+++ src/logic/player.cc	2016-02-18 08:57:58 +0000
@@ -1432,7 +1432,7 @@
  */
 void Player::write_remaining_shipnames(FileWrite & fw) const {
 	fw.unsigned_16(m_remaining_shipnames.size());
-	for (const auto& shipname : m_remaining_shipnames){
+	for (const auto& shipname : m_remaining_shipnames) {
 		fw.string(shipname);
 	}
 }

=== modified file 'src/logic/player.h'
--- src/logic/player.h	2016-02-14 14:09:29 +0000
+++ src/logic/player.h	2016-02-18 08:57:58 +0000
@@ -158,7 +158,7 @@
 		uint32_t last_soldier_trained;
 	} ai_data;
 
-	AiPersistentState* get_mutable_ai_persistent_state(){
+	AiPersistentState* get_mutable_ai_persistent_state() {
 		return &ai_data;
 	}
 

=== modified file 'src/scripting/lua_map.cc'
--- src/scripting/lua_map.cc	2016-02-15 19:03:41 +0000
+++ src/scripting/lua_map.cc	2016-02-18 08:57:58 +0000
@@ -3388,7 +3388,7 @@
 		EditorGameBase & egbase = get_egbase(L);
 		Flag * f = get(L, egbase);
 
-		for (uint32_t i = 1; i <= 6; i++){
+		for (uint32_t i = 1; i <= 6; i++) {
  	       if (f->get_road(i) != nullptr)  {
 				lua_pushstring(L, directions.at(i - 1));
 				upcasted_map_object_to_lua(L, f->get_road(i));
@@ -3862,7 +3862,7 @@
 	if (is_a(Game, &egbase)) {
 		PortDock* pd = get(L, egbase)->get_portdock();
 		if (pd) {
-			if (pd->expedition_started()){
+			if (pd->expedition_started()) {
 				return 1;
 			}
 		}
@@ -3956,7 +3956,7 @@
 		if (!pd) {
 			return 0;
 		}
-		if (!pd->expedition_started()){
+		if (!pd->expedition_started()) {
 			game->send_player_start_or_cancel_expedition(*wh);
 			return 1;
 		}
@@ -3987,7 +3987,7 @@
 			if (!pd) {
 				return 0;
 			}
-		if (pd->expedition_started()){
+		if (pd->expedition_started()) {
 			game->send_player_start_or_cancel_expedition(*wh);
 			return 1;
 		}
@@ -4543,7 +4543,7 @@
 	if (upcast(Game, game, &egbase)) {
 		Ship* ship = get(L, egbase);
 		std::string dir = luaL_checkstring(L, 3);
-		if (dir == "ccw"){
+		if (dir == "ccw") {
 			 game->send_player_ship_explore_island(*ship,  IslandExploreDirection::kCounterClockwise);
 		} else if (dir == "cw") {
 			 game->send_player_ship_explore_island(*ship, IslandExploreDirection::kClockwise);